-
15. Re: Calling Synchronization.afterCompletion() more than once
kconner Mar 14, 2007 2:40 PM (in response to brian.stansberry)"bstansberry@jboss.com" wrote:
All the stuff about Reapable and ReaperThread is what led me to think this wasn't a long-term leak in JBossTS.
The coorindator should have been removed from the TransactionReaper when commit/abort were called. At which point was this reference graph produced? -
16. Re: Calling Synchronization.afterCompletion() more than once
brian.stansberry Mar 14, 2007 2:51 PM (in response to brian.stansberry)"Kevin.Conner@jboss.com" wrote:
At which point was this reference graph produced?
The test makes several independent calls on CMT session beans with default semantics, so for each call the tx should be committed on method exit.
The test then undeploys the deployment.
The graph is produced after the undeployment. Before it's produced numerous attempts to force release of a WeakReference to the classloader are made, so it's probably a good 15-20 seconds after the undeploy completes before the heap snapshot is taken. -
17. Re: Calling Synchronization.afterCompletion() more than once
brian.stansberry Mar 14, 2007 2:54 PM (in response to brian.stansberry)Also, I can eliminate the classloader leak by clearing state in the synchronization's afterCompletion(), so that tells me afterCompletion() is being invoked; i.e. it's not a case of a tx not being committed/rolled back.
-
18. Re: Calling Synchronization.afterCompletion() more than once
kconner Mar 14, 2007 2:56 PM (in response to brian.stansberry)I'm downloading 4.2 now and will try to reproduce it this evening.
-
19. Re: Calling Synchronization.afterCompletion() more than once
marklittle Mar 14, 2007 4:57 PM (in response to brian.stansberry)"Kevin.Conner@jboss.com" wrote:
"mark.little@jboss.com" wrote:
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.
I'm not happy to make such a change in code that has worked successfully for a long time unless it's strictly necessary: so far I'm not convinced it is. AtomicAction is exposed outside of the implementation allowing for recovery hooks with 3rd parties (see previous entry). Changing that behaviour can have significant knock on effects. -
20. Re: Calling Synchronization.afterCompletion() more than once
marklittle Mar 14, 2007 5:08 PM (in response to brian.stansberry)OK, I believe I have found the problem. TransactionImple terminates the transaction via the TwoPhaseCoordinator methods end and cancel. These do not affect the TransactionReaper: only the AtomicAction methods commit and abort do that. Hence the transaction remains referenced by the reaper until the timeout goes off, at which point the reaper will discard it silently. So on the good side, the leak only exists for the duration of the original timeout. On the bad side, there's a leak.
I'll take a look at this tomorrow to see how we fix it. It's not as simple as terminating via AA versus TPC.
Caveat: I'm looking at the JBossTS 4.1 codebase, but I suspect this is the same.