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?

On Thu, Dec 12, 2013 at 12:52 AM, David M. Lloyd <david.lloyd(a)redhat.com&gt; 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(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev

On 12/12/2013 10:35 AM, Sanne Grinovero wrote: > It all looks so much more complex than getting rid of this ThreadLocal? I would like to remove it but, by the comments, it looks like an optimization. It is keeping 6 Marshall and 6 UnMarshall instances to avoid creating them each time you need to (un)marshall an object.