[infinispan-dev] AS clustered classloader leak hunt and web thread local storage keeping reference to a org.infinispan.context.impl.LocalTxInvocationContext ...

Tue Nov 16 09:20:11 EST 2010

On Tue, 2010-11-16 at 10:09 +0100, Galder Zamarreño wrote:
> Hmmm, there's something I don't get here. Who was calling icc.suspend() before your change? I can only see references to it from CacheNotifierImpl and this was done to suspend tx while listener ops were called. So, I'm trying to figure out where those suspend calls you're refering to where coming from.

No one was calling icc.suspend at the end.  That is the bug.  :)

The other calls to suspend were all of the nature of suspending the
active transaction, so non-transactional (or separate transaction) work
can occur.  For that pattern, we always resume the transaction after the
separate task is performed.  I understand this now.  Before, I just knew
that a classloader was kept in memory but had no idea why that was
happening.  :)

One example of
org.infinispan.notifications.cachelistener.CacheNotifierImpl.notifyCacheEntryModified doing this is here:  http://pastie.org/1302567

I also see the same thing for notifyCacheEntryRemoved.

> 
> At first glance, the solution clear the TL at commit/suspend looks simpler than the synchronization callback one. MIrcea, care to comment? 
> 

I'll request a pull of this change then.  :)

If it expands into a synchronization callback, that is fine too.  I'll
wait a bit more but will start cleaning up my changes.

Thanks,
Scott

> On Nov 16, 2010, at 6:06 AM, Scott Marlow wrote:
> 
> > I created ISPN-772 for this issue and have a working patch that cures
> > the TLS leak.  
> > 
> > I changed commit & rollback methods to call icc.suspend(). icc.suspend
> > is currently the only way to clear the invocation context thread local
> > storage. With the current patch, the AS clustered class loader leak test
> > failures are fixed.
> > 
> > One alternative would be to register a synchronization callback to
> > handle calling icc.suspend after the transaction ends. It looks like
> > InvocationContextContainerImpl has a reference to the TM but that might
> > be null (InvocationContextContainerImpl.getRunningTx() tests tm for
> > null).  If anyone knows more, let me know...
> > 
> > If we do use a sync callback, possible code sites to adapt would be:
> > 
> > InvocationContextContainerImpl.createInvocationContext()
> > InvocationContextContainerImpl.createTxInvocationContext()
> > 
> > The other creation methods might be more difficult.  For the remote case
> > (createRemoteTxInvocationContext), would have to look into the best way
> > to handle.
> > 
> > For the local createNonTxInvocationContext, would have to look into the
> > best way to handle.  
> > 
> > It would be good to avoid leaking TLS objects for the more difficult
> > cases but I also don't want to break stuff with my fix.  What do you
> > think?
> > 
> > Thanks,
> > Scott
> > 
> > On Mon, 2010-11-15 at 14:11 -0500, Scott Marlow wrote:
> >> I'm seeing the following:
> >> 
> >> 1.  InvocationContextContainerImpl.createInvocationContext creates a new
> >> LocalTxInvocationContext (lets call it #3011) and sets thread local
> >> (icTL) to it.
> >> 
> >> 2.  We InvocationContextContainerImpl.suspend() LocalTxInvocationContext
> >> #3011 which clears the thread local reference.
> >> 
> >> 3.  We InvocationContextContainerImpl.resume() LocalTxInvocationContext
> >> #3011 which sets the thread local reference.
> >> 
> >> 4.  We InvocationContextContainerImpl.suspend() LocalTxInvocationContext
> >> #3011 which clears the thread local reference.
> >> 
> >> 5.  We InvocationContextContainerImpl.resume() LocalTxInvocationContext
> >> #3011 which sets the thread local reference.
> >> 
> >> I don't see another suspend call, so LocalTxInvocationContext #3011 is
> >> still referenced by the thread local InvocationContextContainerImpl.icTL
> >> 
> >> I'll create a ISPN jira for this. :)
> >> 
> >> 
> >> Scott
> >> 
> >> On Mon, 2010-11-15 at 13:12 -0500, Scott Marlow wrote:
> >>> I'm getting a little closer, should be closer sooner :)
> >>> 
> >>> The LocalTxInvocationContext that got left in thread local storage was
> >>> instantiated from here http://pastie.org/1300157
> >>> 
> >>> That should bring me closer to the cause.  
> >>> 
> >>> On Mon, 2010-11-15 at 10:29 -0500, Scott Marlow wrote:
> >>>> Hi,
> >>>> 
> >>>> I'm trying to chase down some memory leaks for
> >>>> https://jira.jboss.org/browse/JBAS-8613 (turned into a catch all jira
> >>>> for a few different AS clustering failures).
> >>>> 
> >>>> I captured a picture of a classloader leak that is attached to
> >>>> JBAS-8613.  
> >>>> 
> >>>> The http thread threadLocals has a
> >>>> org.infinispan.context.impl.LocalTxInvocationContext that is keeping the
> >>>> classloader in memory (via what it references which is shown in the
> >>>> abovce link or more directly
> >>>> https://jira.jboss.org/secure/attachment/12338702/Eclipse+Memory
> >>>> +Analyzer+Tool.png)
> >>>> 
> >>>> I'm looking at a different problem at the moment but would like to fix
> >>>> this leak soon.
> >>>> 
> >>>> Why would org.infinispan.context.impl.LocalTxInvocationContext still be
> >>>> referenced in thread local storage?  Any suggestions on specific classes
> >>>> that should be looked at?
> >>>> 
> >>>> Thanks,  
> >>>> Scott
> >>>> 
> >>>> _______________________________________________
> >>>> infinispan-dev mailing list
> >>>> infinispan-dev at lists.jboss.org
> >>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
> >>> 
> >>> 
> >>> _______________________________________________
> >>> infinispan-dev mailing list
> >>> infinispan-dev at lists.jboss.org
> >>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
> >> 
> >> 
> >> _______________________________________________
> >> infinispan-dev mailing list
> >> infinispan-dev at lists.jboss.org
> >> https://lists.jboss.org/mailman/listinfo/infinispan-dev
> > 
> > 
> > _______________________________________________
> > infinispan-dev mailing list
> > infinispan-dev at lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/infinispan-dev
> 
> --
> Galder Zamarreño
> Sr. Software Engineer
> Infinispan, JBoss Cache
> 