[jbossts-issues] [JBoss JIRA] (JBTM-770) incorrect cleanup registration causes memory leak

[Jonathan Halliday (Commented) (JIRA)]

    [ https://issues.jboss.org/browse/JBTM-770?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12646634#comment-12646634 ] 

Jonathan Halliday commented on JBTM-770:
----------------------------------------

Actually they are both wrong, but the bug in the 4.6.1 version is not relevant to AS use cases.

problem = !super.afterCompletion(myStatus == Status.StatusCommitted ? ActionStatus.COMMITTED : ActionStatus.ABORTED);

incorrectly ignores the prior value of problem, such that an exception won't be thrown in some cases where it should be. However, since ServerTransaction just throws it away anyhow that's not a big issue.

problem = problem || !super.afterCompletion(myStatus == Status.StatusCommitted ? ActionStatus.COMMITTED : ActionStatus.ABORTED);

attempts to preserve the prior value of problem, but fails to account for short circuit boolean evaluation. As a result the parent afterCompletions won't be called in some cases, which is more serious.

The correct form is arguably

boolean superProblem = !super.afterCompletion(myStatus == Status.StatusCommitted ? ActionStatus.COMMITTED : ActionStatus.ABORTED);
problem = problem || superProblem


                
> incorrect cleanup registration causes memory leak
> -------------------------------------------------
>
>                 Key: JBTM-770
>                 URL: https://issues.jboss.org/browse/JBTM-770
>             Project: JBoss Transaction Manager
>          Issue Type: Bug
>      Security Level: Public(Everyone can see) 
>          Components: JTS
>    Affects Versions: 4.12.0, 4.6.1.CP06
>            Reporter: Jonathan Halliday
>            Assignee: Jonathan Halliday
>             Fix For: 4.13.0, 4.6.1.CP08
>
>
> jts.TransactionImple constructors attempt to optimize the cleanup callback needed to remove entries from _transactions, bypassing the ORB for interposition scenarios:
> theTx = (TwoPhaseCoordinator) BasicAction.Current();
> if (theTx != null) {
>   theTx.addSynchronization(new LocalCleanupSynchronization(this));
> } else {
>   registerSynchronization(new CleanupSynchronization(this));
> }
> Unfortunately this does not work and leads to memory leaks on _transactions in certain cases. Specifically, where theTx is a ServerTransaction (i.e. extends ArjunaTransactionImple, which extends TwoPhaseCoordinator), the callbacks run (by ArjunaTransactionImple.do[Before|After]Completion()) are those in ArjunaTransactionImple._syncs, not the masked ones in TwoPhaseCoordinator._syncs. Thus using TwoPhaseCoordinator.addSynchronization registers a callback which is never invoked.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.jboss.org/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira