[infinispan-dev] Clean ThreadLocals

[Sanne Grinovero]

It all looks so much more complex than getting rid of this ThreadLocal?

On 12 December 2013 09:01, Dan Berindei <dan.berindei at gmail.com> wrote:
>
>
>
> 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
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev