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

[Dan Berindei]

On Fri, May 13, 2011 at 5:48 PM, Jason T. Greene
<jason.greene at redhat.com> wrote:
> On 5/12/11 4:18 AM, Dan Berindei wrote:
>>
>> On Wed, May 11, 2011 at 11:18 PM, David Bosschaert<david at redhat.com>
>>  wrote:
>>>
>>> On 11/05/2011 17:54, Dan Berindei wrote:
>>>>
>>>> On Wed, May 11, 2011 at 7:08 PM, Pete Muir<pmuir at redhat.com>    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().
>
> This approach just doesn't scale, and it would not work in all environments
> (there is no gaurantee you can get the original classloader if the
> environment doesn't allow you to look them up by some kind of unique id).
>
>> 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
>
> Yes thread locals are brittle, and tend to leak if they aren't managed
> correctly. Using a context specific instance is a much better approach.
>
>> 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.
>>
>
> That's similar to what I was proposing with the CacheSession notion. The
> basic notion is that you move thread/context specific state to a separate
> object that the caller uses (optionaly), and it mirrors the same Cache API.
>

The way I understood your proposal was that the session would be
something visible to the user, so he would have to call
cacheManager.getCache("A").getSession() in order to access the context
functionality.
My version essentially the same thing, except the session would be
created transparently when the user calls usingClassLoader() or
withFlags().

>> 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.
>
> You dont need to reuse the same impl. Just create a new delegate which
> passes an extra invocation state parameter with ever call. The overhead of
> that is tiny.
>

Some of the methods in CacheDelegate do have lots of logic in them, so
we have to reuse them. The way I see it the existing methods will
change to receive the context parameter explicitly instead of
implicitly, and the methods with the old signature will call them with
a default context. usingClassLoader/withFlags will create an instance
of CacheDelegateSession, and the methods in CacheDelegateSession will
modify/use the context stored in that instance.

I agree the overhead is tiny, and the best thing is users can optimize
it away by moving the usingClassLoader/withFlags calls outside of
loops.

Cheers
Dan