Calling Synchronization.afterCompletion() more than once

[brian.stansberry]

Can a Synchronization's afterCompletion() method be called more than once, e.g in a recovery scenario or something?

I'm wondering because I wrote some unit tests that check for classloader leaks following an undeployment. If transactions are used, test shows reference chains leading back to JBossTS via a Synchronization. My assumption is JBossTS releases the ref to the Synchronization in some later cleanup task.

Having the test try to figure out when the TM will clean up the Synchronization ref is pretty unworkable, so a seemingly simple workaround was to have the Synchronization null out its state in afterCompletion(). But that's not good if afterCompletion() can potentially be called again.

[kconner]

Synchronisations are not persisted for recovery, only resources, so there is no danger of them being called a second time.

The synchronisations should be cleared after the transaction processing is completed though, are you not seeing this?

[jhalliday]

Sync's are not persisted, so don't get called at all in a recovery scenario if the JVM containing them crashed. In non-crash cases they should be called exactly once. In looking at the code, the collection used to hold the set of Syncs registered for a particular tranaction is explicitly nulled out in the end of afterCompletion processing. However, there is another reference to just one Sync (the last one processed in beforeCompletion) that is not nulled. It therefore won't be colleced until the containing transaction is, although that should not be a problem. I'll add code to explicitly null it out anyhow.

Jonathan

[marklittle]

Just an addendum: from a protocol perspective there's no guarantee that afterCompletions will ever be called. They are a courtesy call and failure to produce them won't have an affect on the transaction.

[kconner]

Jonathan, it looks like the real issue may be that a reference to the coordinator is still held by the transaction after the transaction has terminated.

The transaction impl should disassociate itself from the coordinator on completion.

[marklittle]

Jonathan, it looks like the real issue may be that a reference to the coordinator is still held by the transaction after the transaction has terminated.

The transaction impl should disassociate itself from the coordinator on completion.

The transaction is garbage collected when it completes.

[marklittle]

I'm wondering because I wrote some unit tests that check for classloader leaks following an undeployment. If transactions are used, test shows reference chains leading back to JBossTS via a Synchronization. My assumption is JBossTS releases the ref to the Synchronization in some later cleanup task.

Are you using the JTA or the JTS implementation from JBossTS?

[kconner]

The transaction is garbage collected when it completes.

Unless a reference is held elsewhere which is why the transaction should disassociate itself from the coordinator on completion.

[marklittle]

I'm wondering because I wrote some unit tests that check for classloader leaks following an undeployment. If transactions are used, test shows reference chains leading back to JBossTS via a Synchronization. My assumption is JBossTS releases the ref to the Synchronization in some later cleanup task.

What Synchronization? Are you saying that for each transaction, a Synchronization implementation isn't being garbage collected or that other objects from JBossTS aren't being garbage collected? I'm unclear from your description.

[marklittle]

The transaction is garbage collected when it completes.

Unless a reference is held elsewhere which is why the transaction should disassociate itself from the coordinator on completion.

If JCA is involved then there was an issue a month or so ago that could have caused a leak, but that's been fixed and was in time for the CR2 build, I believe. The transaction does disassociate itself from the global (VM) repository when it terminates and everything else that has a reference to the transaction is also transient, with the exception of the transaction reaper. However, terminated transactions also remove themselves from there too.

[kconner]

We cannot control who has a reference to a transaction as it is exposed to code outside of TS. If there is any code which has a reference to the transaction impl, for whatever reason, then all instances referenced by that transaction cannot be garbaged collected.

There is no reason for the transaction impl to keep a reference to the coordinator and associated objects once the transaction has completed. We should disassociate on completion.

[marklittle]

The current implementation assumes that the AtomicAction instance is around after termination for status, identification and in the event of heuristics potentially for management and recovery information (the log still exists on disk at that point and hence the AA instance can be used to access it). Changing it so that is no longer the case could have implications throughout the areas of the code. I'd prefer to see what is keeping hold of the TransactionImple reference and not releasing that, since it appears that is the root of the leak (assuming we're understanding the reported problem.)

[brian.stansberry]

I'm wondering because I wrote some unit tests that check for classloader leaks following an undeployment. If transactions are used, test shows reference chains leading back to JBossTS via a Synchronization. My assumption is JBossTS releases the ref to the Synchronization in some later cleanup task.

What Synchronization? Are you saying that for each transaction, a Synchronization implementation isn't being garbage collected or that other objects from JBossTS aren't being garbage collected? I'm unclear from your description.

Using JBossTS; AS Branch_4_2. The specific Synchronization was org.jboss.ejb.plugins.StatefulSessionInstanceInterceptor$InstanceSynchronization (used in EJB2 SFSBs to clear bean locks).

I'll remove the state-nulling bit I did and re-run the test. That will produce a report showing the reference chain; I'll post it.

[brian.stansberry]

Here's the reference chain from the classloader back into JBossTS. There's a lot more besides this branching off from the com.arjuna.ats.arjuna.coordinator.TransactionReaper._list ref, but this is the gist of it. I can e-mail someone the full report if it's wanted.

!--org.jboss.mx.loading.UnifiedClassLoader3@1085eba{ url=null ,addedOrder=49}
!--!--ClassLoaderReference @ class $Proxy80
!--!--!--InstanceOfReference:ToString=ClassloaderLeakStatefulSession:eza31jqw-8
!--!--!--!--FieldReference [e-mail]eContext@13ba312=org.jboss.ejb.StatefulSessionEnterpriseContext@13ba312
!--!--!--!--!--FieldReference private org.jboss.ejb.EnterpriseContext org.jboss.ejb.plugins.StatefulSessionInstanceInterceptor$InstanceSynchronization.ctx=org.jboss.ejb.plugins.StatefulSessionInstanceInterceptor$InstanceSynchronization@122be0b
!--!--!--!--!--!--FieldReference private javax.transaction.Synchronization com.arjuna.ats.internal.jta.resources.arjunacore.SynchronizationImple._theSynch=com.arjuna.ats.internal.jta.resources.arjunacore.SynchronizationImple@ffa9d8
!--!--!--!--!--!--!--FieldReference private com.arjuna.ats.arjuna.coordinator.SynchronizationRecord com.arjuna.ats.arjuna.coordinator.TwoPhaseCoordinator._currentRecord=BasicAction: a0b0e66:58c:45f838a3:41 status: ActionStatus.COMMITTED
!--!--!--!--!--!--!--!--FieldReference public com.arjuna.ats.arjuna.coordinator.Reapable com.arjuna.ats.internal.arjuna.coordinator.ReaperElement._control=com.arjuna.ats.internal.arjuna.coordinator.ReaperElement@c47220
!--!--!--!--!--!--!--!--!--FieldReference private final com.arjuna.ats.internal.arjuna.template.OrderedListElement com.arjuna.ats.internal.arjuna.template.OrderedListEntry._theElement=com.arjuna.ats.internal.arjuna.template.OrderedListEntry@16e2b70
!--!--!--!--!--!--!--!--!--!--FieldReference private com.arjuna.ats.internal.arjuna.template.OrderedListEntry com.arjuna.ats.internal.arjuna.template.OrderedListEntry._next=com.arjuna.ats.internal.arjuna.template.OrderedListEntry@1e5d007
!--!--!--!--!--!--!--!--!--!--!--FieldReference private com.arjuna.ats.internal.arjuna.template.OrderedListEntry com.arjuna.ats.internal.arjuna.template.OrderedListEntry._next=com.arjuna.ats.internal.arjuna.template.OrderedListEntry@bc8f01
!--!--!--!--!--!--!--!--!--!--!--!--FieldReference private com.arjuna.ats.internal.arjuna.template.OrderedListEntry com.arjuna.ats.internal.arjuna.template.OrderedListEntry._next=com.arjuna.ats.internal.arjuna.template.OrderedListEntry@1509443
!--!--!--!--!--!--!--!--!--!--!--!--!--FieldReference private com.arjuna.ats.internal.arjuna.template.OrderedListEntry com.arjuna.ats.internal.arjuna.template.OrderedList._headOfList=com.arjuna.ats.internal.arjuna.template.OrderedList@1c931fb
!--!--!--!--!--!--!--!--!--!--!--!--!--!--FieldReference private com.arjuna.ats.internal.arjuna.template.OrderedList com.arjuna.ats.arjuna.coordinator.TransactionReaper._list=com.arjuna.ats.arjuna.coordinator.TransactionReaper@126c885
!--!--!--!--!--!--!--!--!--!--!--!--!--!--!--Root
!--!--!--!--!--!--!--!--!--!--!--!--!--!--!--Reference inside a method - com.arjuna.ats.internal.arjuna.coordinator.ReaperThread::run
!--!--!--!--!--!--!--!--!--!--!--!--!--!--!--FieldReference private com.arjuna.ats.arjuna.coordinator.TransactionReaper com.arjuna.ats.internal.arjuna.coordinator.ReaperThread.reaperObject=Thread[Thread-23,5,jboss]
!--!--!--!--!--!--!--!--!--!--!--!--!--!--!--!--Root
!--!--!--!--!--!--!--!--!--!--!--!--!--!--!--!--Reference inside a method - com.arjuna.ats.internal.arjuna.coordinator.ReaperThread::run

All the stuff about Reapable and ReaperThread is what led me to think this wasn't a long-term leak in JBossTS.

For more info on the test that produces this, see http://wiki.jboss.org/wiki/Wiki.jsp?page=ClassloaderLeakUnitTestCase

[kconner]

The current implementation assumes that the AtomicAction instance is around after termination for status, identification and in the event of heuristics potentially for management and recovery information (the log still exists on disk at that point and hence the AA instance can be used to access it). Changing it so that is no longer the case could have implications throughout the areas of the code. I'd prefer to see what is keeping hold of the TransactionImple reference and not releasing that, since it appears that is the root of the leak (assuming we're understanding the reported problem.)

No reference to the internal coordinator appears to exist outside of the transactionimpls and their associated transaction manager.

The majority of the methods in the base transactionimpl classes assumes that this reference can be nulled, the only unsafe methods appear to be equals(jts) and hashCode (because their results can change) and also get_uid. The subordinate transactionimpls would also have to be change to check for a null coordinator.