Re: [infinispan-dev] [Pull Request] Modular Classloading Compatibility

On 16 May 2011, at 18:20, Galder Zamarreño wrote: > > On May 12, 2011, at 11:18 AM, Dan Berindei wrote: > >> On Wed, May 11, 2011 at 11:18 PM, David Bosschaert <david(a)redhat.com&gt; wrote: >>> On 11/05/2011 17:54, Dan Berindei wrote: >>>> On Wed, May 11, 2011 at 7:08 PM, Pete Muir&lt;pmuir(a)redhat.com&gt;  wrote: >>>>> Were we developing for OSGi I would certainly agree with you. However in many environments today we can reasonably expect the TCCL to be set and to be able to load the classes we need. So whilst making it part of the API is the safest option, it's also making complicated an API for the sake of the few at the cost of the many. Further this also seems kinda nasty to me. We know the class (and hence bundle/module) when we put the object into Infinispan, therefore why do we require people to respecify this again? >>>>> >>>>> David, can we not actually do something here akin to what we are discussing for Weld? Whereby we can serialize out the bundle id and then find the correct CL based on that when we deserialize. >>>> What if the object is a java.util.ArrayList? Each element in the list >>>> could belong to a different bundle, so you'd have to write a bundle id >>>> for every element in the list. >>> Yes, if you know the Bundle-SymbolicName and Version (or the Bundle ID) >>> you can find its classloader. >>> >>> On the other question, if you're passing in a class object then you can >>> obtain its classloader and hence the bundle where it came from. But, and >>> I think this is what Dan allused to above, is it always true that the >>> class your passing in comes from the bundle that you need to have or >>> could it also come from one of its parent class loaders? >>> >> >> Exactly David, sorry if my message was a little cryptic. I think in >> order to handle every case properly you would have to go through the >> entire object graph being stored in the cache in order to find all the >> classloaders/bundle ids that you will need on get(). >> >> That seems like a lot of overhead to me, and forcing the user to >> provide the classloader doesn't seem that bad in comparison. Perhaps >> we should use something other than a thread-local for this though, so >> that users can do a onto the result of a >> cacheManager.getCache("A").usingClassLoader(A.class) and never have to >> provide the classloader again. >> >> In fact I think this is a good idea for the invocation flags we >> already have, too. It would involve creating lots of overloads in >> CacheDelegate with a PreInvocationContext parameter and a new >> CacheDelegateWithContext class to invoke those methods, but the public >> API would remain the same. > > No matter how I look at it, putting a classloader in a thread local makes me shiver. I also wonder why we want do this, given we already have a construct called the Thread Local Context Classloader ;-) Either we use that, or use some other mechanism. > Just imagine the mayhem you can cause if you "forget" to clear the thread local. > > I've done enough of Apache Commons Logging support to understand that you should limit the references to classloaders to the minimum, particularly in system classes/infrastructure. > > If we need to end up forcing users to register classloaders in these scenarions, we need to do it in such way that either: > > - we can detect these leaks (it might be a bit primitive now but old JBoss JCA code had an interesting way of discovering unclosed open connections) > > - if we give you on trying to detect them, the impact of a leak is reduced as much as possible. > > Cheers, > -- > Galder Zamarreño > Sr. Software Engineer > Infinispan, JBoss Cache > > > _______________________________________________ > 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