-
Notifications
You must be signed in to change notification settings - Fork 46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WFTC-136] Memory leak, remove XAResourceRecovery when reload #172
Conversation
08bce10
to
42240ab
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes this is a nice fix and I definitely approve of it!
src/main/java/org/wildfly/transaction/client/provider/jboss/JBossLocalTransactionProvider.java
Show resolved
Hide resolved
|
||
public void removeXAResourceRecovery(XAResourceRecoveryRegistry registry) { | ||
registry.removeXAResourceRecovery(xaResourceRecovery); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Superfluous blank line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
42240ab
to
8733df8
Compare
@@ -77,6 +78,7 @@ public abstract class JBossLocalTransactionProvider implements LocalTransactionP | |||
private final ConcurrentSkipListSet<XidKey> timeoutSet = new ConcurrentSkipListSet<>(); | |||
private final ConcurrentMap<SimpleXid, Entry> known = new ConcurrentHashMap<>(); | |||
private final FileSystemXAResourceRegistry fileSystemXAResourceRegistry; | |||
private XAResourceRecovery xaResourceRecovery; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this field can be final.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @chengfang for your review! I addressed your comment.
src/main/java/org/wildfly/transaction/client/provider/jboss/JBossLocalTransactionProvider.java
Show resolved
Hide resolved
8733df8
to
c30e7b3
Compare
all 6 checks passed. |
@marcosgopen @mmusgrov thanks! |
https://issues.redhat.com/browse/WFTC-136