[infinispan-dev] Clean ThreadLocals

[Dan Berindei]

On Thu, Dec 12, 2013 at 12:52 AM, David M. Lloyd <david.lloyd at redhat.com>wrote:

> On 12/11/2013 04:47 PM, Pedro Ruivo wrote:
> > Hi,
> >
> > I've created a method to clean a specific ThreadLocal variable from all
> > live threads [1].
> >
> > My goal is to clean the ThreadLocal variables after a cache stops. It's
> > kind expensive method (it uses reflection) but I think it is fine.
> >
> > Currently, I'm using it to clear the Marshaller ThreadLocal in here [2].
> >
> > Does anyone see any problem with this approach? Maybe it can be used in
> > other possible leaking ThreadLocals?
>
> It's an interesting idea (I've done something similar in the past to
> simply drop all thread locals).  I would recommend that you check that
> all the JDKs you want to support use the same technique for thread
> locals though.  Because these fields are not part of the JDK, they may
> not always exist in all environments.
>

Yeah, the implementation of ThreadLocal has changed in the past and is
likely to change again in the future. If that happens and we have to
support different JDKs with different methods for clearing ThreadLocals, it
won't be pretty.


> Also, it is important to only ever clean the thread locals of your
> current thread, or you're inviting all kinds of bizarre concurrency
> problems (maybe even locking threads into infinite loops), especially if
> the thread is running and actively using thread locals at the time.
>

Indeed, ThreadLocalMap doesn't look to be thread-safe. I'm not sure if a
remote() from another thread can cause an infinite loop like in HashMap
because it uses open addressing instead of chaining, but it looks like it
may cause a get() for a different ThreadLocal to return the wrong instance.
(I would be ok with it if it would only cause breakage for threads using
the cache that's being stopped, but it looks like it can touch more than
one entry.)

I think we shouldn't concern ourselves with actually removing the
ThreadLocal from the map, we just have to keep track of the ThreadLocal
instances and clean any references to other objects they might have (a la
ExternalizerTableProxy). Or maybe give up on ThreadLocals and use a
ConcurrentWeakKeyHashMap<Thread, PerThreadInstanceHolder> instead.

Dan



> --
> - DML
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/infinispan-dev/attachments/20131212/d2260aa7/attachment.html