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

[Pete Muir]

Galder commented this on Github, which I think the answer to deserves a wider 
audience:

>> @@ -155,9 +155,9 @@ public class ConfigFilesConvertor {
>>   }
>> 
>>   if (type.equals(JBOSS_CACHE3X)) {
>> - transformFromJbossCache3x(sourceName, destinationName);
>> + transformFromJbossCache3x(sourceName, destinationName, 
>> ConfigFilesConvertor.class.getClassLoader());

Actually this is a bug, I can just pass null here (see below).

> 
> Just a question to clarify my understanding: So here since the XSLT to load 
> comes from us, we simply use our own cache loader cos the file should be 
> there. For any classes that are loaded coming potentially from the user, use 
> TCCL, right?


If we know a class/resource comes from an Infinispan jar, then it must be on 
the same classloader as the class we are currently loading from, so we 
definitely know the classloader (it's the one used to load us).

For any classes that may come from the application, we need to use the app 
classloader. In a traditional Java EE env, this is the TCCL, however outside of 
such an environment, the TCCL may well not be set, so we need to know about the 
app classloader in some other way. This is where it get's tricky ;-)

The classloader lookup I have gone with in Infinispan is:

* If a (app) classloader is passed in, look here,
* If the class is not found in the app classloader (either not visible or no 
app classloader) look on the Infinispan classloader
* Otherwise, look on the system classloader (some comment about agent based 
instrumentation means we apparently need this)

At the moment, I have always passed in the TCCL as the app classloader at the 
"furthest out" point that I can reach, and I now need to go through the code 
base and remove all refs to the TCCL, using (a), (b), (c) below.

On 8 Jun 2011, at 14:13, Manik Surtani wrote:

> 
> On 8 Jun 2011, at 14:06, Pete Muir wrote:
> 
>> We've decided that rather than swap out the TCCL (which is frankly a bit 
>> error prone), to fix this properly and have it so that each time a class is 
>> loaded it must select the classloaders it wants. To help, we will add an 
>> advancedCache.getClassLoader(), which defaults to the TCCL but can be 
>> overridden as described below.
>> 
>> As a first step, I have removed the TCCL from the default lookup in Util 
>> (AFAICT all class loading was going through there), and required that, if 
>> you want to lookup app classes (as opposed to Infinispan classes) you 
>> explicitly pass in a classloader. I have then passed in the TCCL as a 
>> parameter throughout the codebase. This now makes it explicit where the TCCL 
>> is being used and should mean that any new work does think about whether 
>> they need to look at app classes or not.
>> 
>> Under this new scheme we have 45 refs to the TCCL in the codebase. Some of 
>> these are:
>> 
>> a) not needed, we are only ever loading system classes
>> b) can, with a bit of a refactor, refer to the classloader we select using 
>> withClassLoader()
>> c) other (these will be harder to fix :-()
>> 
>> My plan is to go through each one, and chat with the owner of the code and 
>> discuss which of (a), (b) or (c) is relevant here.
> 
> Sounds good.
> 
> --
> Manik Surtani
> ma...@jboss.org
> twitter.com/maniksurtani
> 
> Lead, Infinispan
> http://www.infinispan.org
> 
> 
> 
> 
> ___
> infinispan-dev mailing list
> infinispan-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev


___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Manik Surtani]

On 8 Jun 2011, at 14:06, Pete Muir wrote:

> We've decided that rather than swap out the TCCL (which is frankly a bit 
> error prone), to fix this properly and have it so that each time a class is 
> loaded it must select the classloaders it wants. To help, we will add an 
> advancedCache.getClassLoader(), which defaults to the TCCL but can be 
> overridden as described below.
> 
> As a first step, I have removed the TCCL from the default lookup in Util 
> (AFAICT all class loading was going through there), and required that, if you 
> want to lookup app classes (as opposed to Infinispan classes) you explicitly 
> pass in a classloader. I have then passed in the TCCL as a parameter 
> throughout the codebase. This now makes it explicit where the TCCL is being 
> used and should mean that any new work does think about whether they need to 
> look at app classes or not.
> 
> Under this new scheme we have 45 refs to the TCCL in the codebase. Some of 
> these are:
> 
> a) not needed, we are only ever loading system classes
> b) can, with a bit of a refactor, refer to the classloader we select using 
> withClassLoader()
> c) other (these will be harder to fix :-()
> 
> My plan is to go through each one, and chat with the owner of the code and 
> discuss which of (a), (b) or (c) is relevant here.

Sounds good.

--
Manik Surtani
ma...@jboss.org
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org




___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Pete Muir]

We've decided that rather than swap out the TCCL (which is frankly a bit error 
prone), to fix this properly and have it so that each time a class is loaded it 
must select the classloaders it wants. To help, we will add an 
advancedCache.getClassLoader(), which defaults to the TCCL but can be 
overridden as described below.

As a first step, I have removed the TCCL from the default lookup in Util 
(AFAICT all class loading was going through there), and required that, if you 
want to lookup app classes (as opposed to Infinispan classes) you explicitly 
pass in a classloader. I have then passed in the TCCL as a parameter throughout 
the codebase. This now makes it explicit where the TCCL is being used and 
should mean that any new work does think about whether they need to look at app 
classes or not.

Under this new scheme we have 45 refs to the TCCL in the codebase. Some of 
these are:

a) not needed, we are only ever loading system classes
b) can, with a bit of a refactor, refer to the classloader we select using 
withClassLoader()
c) other (these will be harder to fix :-()

My plan is to go through each one, and chat with the owner of the code and 
discuss which of (a), (b) or (c) is relevant here.

On 8 Jun 2011, at 10:48, Manik Surtani wrote:

> Apologies for my long absence from this thread.
> 
> On 20 May 2011, at 15:28, Jason T. Greene wrote:
> 
>> On 5/18/11 11:06 AM, Manik Surtani wrote:
>> 
>>> 
>>> 1) Class loader per session/cache.
>>> 
>>> I like Jason/Sanne/Trustin's suggestions of a session-like contract, and 
>>> specifically I think this is best achieved as a delegate to a cache, again 
>>> as suggested elsewhere by Pete, etc.  E.g.,
>>> 
>>> Cache  myCache = cacheManager.getCache("myCache", myClassLoader);
>> 
>> -snip-
>> 
>> I would recommend leaving this open to store other per-session configuration 
>> values (perhaps with a builder), and some of them mutable.
>> This will allow you to completely eliminate the ThreadLocal context stuff 
>> used today which is both faster and more robust (the gc will clean up the 
>> state for you).
> 
> Are you suggesting something like:
> 
>   cacheManager.withClassLoader(myClassLoader).getCache("myCache");
> 
> thus allowing future expansions such as:
> 
>   
> cacheManager.withClassLoader(myClassLoader).withSomeOtherParam(param).getCache()
> 
> ?
> 
> I do like this, since it doesn't pollute the basic cache manager API methods 
> like getCache().
> 
>> 
>>> 2) Class loader per invocation.
>>> 
>> 
>> If this "session" notion has some mutable values, you could also make CL 
>> mutable:
>> 
>> cacheSession.setClassLoader(blah);
>> cacheSession.setFlags(FORCE_WRITE_LOCK)
>> 
> 
> Yes, no reason why the delegate Cache can't do this.  These setters would 
> need to exist on the Cache interface though.  For now, we should just 
> restrict to setClassLoader().
> 
>> From an implementation perspective, given where we are with 5.0 now, I 
>> suggest we implement by holding the ClassLoader in the CacheDelegate impl, 
>> and each method invocation impl would:
> 
> 1) Set TCCL with the instance's ClassLoader field
> 2) Do work
> 3) In a finally block, reset TCCL.
> 
> This is just a temp measure, since I don't want to re-work how Marshallers, 
> etc get a hold of the class loader.  They use TCCLs right now, and while 
> sub-optimal in many ways, this approach gives us an easy mechanism to 
> implement while still preventing any leaks, etc otherwise common with TCCLs.
> 
> Cheers
> Manik
> 
> --
> Manik Surtani
> ma...@jboss.org
> twitter.com/maniksurtani
> 
> Lead, Infinispan
> http://www.infinispan.org
> 
> 
> 
> 
> ___
> infinispan-dev mailing list
> infinispan-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev


___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Manik Surtani]

On 8 Jun 2011, at 11:42, Dan Berindei wrote:

>> 
>> Yes, no reason why the delegate Cache can't do this.  These setters would 
>> need to exist on the Cache interface though.  For now, we should just 
>> restrict to setClassLoader().
>> 
>>> From an implementation perspective, given where we are with 5.0 now, I 
>>> suggest we implement by holding the ClassLoader in the CacheDelegate impl, 
>>> and each method invocation impl would:
>> 
>> 1) Set TCCL with the instance's ClassLoader field
>> 2) Do work
>> 3) In a finally block, reset TCCL.
>> 
>> This is just a temp measure, since I don't want to re-work how Marshallers, 
>> etc get a hold of the class loader.  They use TCCLs right now, and while 
>> sub-optimal in many ways, this approach gives us an easy mechanism to 
>> implement while still preventing any leaks, etc otherwise common with TCCLs.
>> 
> 
> The CacheDelegate instance returned by CM.getCache() is shared between
> all the users that called getCache(), so if we want different users to
> have different classloaders I think we need another Cache wrapper
> class that will set/reset the TCCL.


Yes, that's what I have in mind.
--
Manik Surtani
ma...@jboss.org
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org




___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Dan Berindei]

On Wed, Jun 8, 2011 at 12:48 PM, Manik Surtani  wrote:
> Apologies for my long absence from this thread.
>
> On 20 May 2011, at 15:28, Jason T. Greene wrote:
>
>> On 5/18/11 11:06 AM, Manik Surtani wrote:
>>
>>>
>>> 1) Class loader per session/cache.
>>>
>>> I like Jason/Sanne/Trustin's suggestions of a session-like contract, and 
>>> specifically I think this is best achieved as a delegate to a cache, again 
>>> as suggested elsewhere by Pete, etc.  E.g.,
>>>
>>>      Cache  myCache = cacheManager.getCache("myCache", myClassLoader);
>>
>> -snip-
>>
>> I would recommend leaving this open to store other per-session configuration 
>> values (perhaps with a builder), and some of them mutable.
>> This will allow you to completely eliminate the ThreadLocal context stuff 
>> used today which is both faster and more robust (the gc will clean up the 
>> state for you).
>
> Are you suggesting something like:
>
>        cacheManager.withClassLoader(myClassLoader).getCache("myCache");
>
> thus allowing future expansions such as:
>
>        
> cacheManager.withClassLoader(myClassLoader).withSomeOtherParam(param).getCache()
>
> ?
>
> I do like this, since it doesn't pollute the basic cache manager API methods 
> like getCache().
>
>>
>>> 2) Class loader per invocation.
>>>
>>
>> If this "session" notion has some mutable values, you could also make CL 
>> mutable:
>>
>> cacheSession.setClassLoader(blah);
>> cacheSession.setFlags(FORCE_WRITE_LOCK)
>>
>
> Yes, no reason why the delegate Cache can't do this.  These setters would 
> need to exist on the Cache interface though.  For now, we should just 
> restrict to setClassLoader().
>
> >From an implementation perspective, given where we are with 5.0 now, I 
> >suggest we implement by holding the ClassLoader in the CacheDelegate impl, 
> >and each method invocation impl would:
>
> 1) Set TCCL with the instance's ClassLoader field
> 2) Do work
> 3) In a finally block, reset TCCL.
>
> This is just a temp measure, since I don't want to re-work how Marshallers, 
> etc get a hold of the class loader.  They use TCCLs right now, and while 
> sub-optimal in many ways, this approach gives us an easy mechanism to 
> implement while still preventing any leaks, etc otherwise common with TCCLs.
>

The CacheDelegate instance returned by CM.getCache() is shared between
all the users that called getCache(), so if we want different users to
have different classloaders I think we need another Cache wrapper
class that will set/reset the TCCL.


> Cheers
> Manik
>
> --
> Manik Surtani
> ma...@jboss.org
> twitter.com/maniksurtani
>
> Lead, Infinispan
> http://www.infinispan.org
>
>
>
>
> ___
> infinispan-dev mailing list
> infinispan-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>

___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Manik Surtani]

On 20 May 2011, at 15:39, Jason T. Greene wrote:

> So I think to ever justify it, it would take some very convincing 
> numbers that eager deserialization is indeed better.

Agreed.

--
Manik Surtani
ma...@jboss.org
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org



___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Manik Surtani]

Apologies for my long absence from this thread.

On 20 May 2011, at 15:28, Jason T. Greene wrote:

> On 5/18/11 11:06 AM, Manik Surtani wrote:
> 
>> 
>> 1) Class loader per session/cache.
>> 
>> I like Jason/Sanne/Trustin's suggestions of a session-like contract, and 
>> specifically I think this is best achieved as a delegate to a cache, again 
>> as suggested elsewhere by Pete, etc.  E.g.,
>> 
>>  Cache  myCache = cacheManager.getCache("myCache", myClassLoader);
> 
> -snip-
> 
> I would recommend leaving this open to store other per-session configuration 
> values (perhaps with a builder), and some of them mutable.
> This will allow you to completely eliminate the ThreadLocal context stuff 
> used today which is both faster and more robust (the gc will clean up the 
> state for you).

Are you suggesting something like:

cacheManager.withClassLoader(myClassLoader).getCache("myCache");

thus allowing future expansions such as:


cacheManager.withClassLoader(myClassLoader).withSomeOtherParam(param).getCache()

?

I do like this, since it doesn't pollute the basic cache manager API methods 
like getCache().

> 
>> 2) Class loader per invocation.
>> 
> 
> If this "session" notion has some mutable values, you could also make CL 
> mutable:
> 
> cacheSession.setClassLoader(blah);
> cacheSession.setFlags(FORCE_WRITE_LOCK)
> 

Yes, no reason why the delegate Cache can't do this.  These setters would need 
to exist on the Cache interface though.  For now, we should just restrict to 
setClassLoader().

>From an implementation perspective, given where we are with 5.0 now, I suggest 
>we implement by holding the ClassLoader in the CacheDelegate impl, and each 
>method invocation impl would:

1) Set TCCL with the instance's ClassLoader field
2) Do work
3) In a finally block, reset TCCL.

This is just a temp measure, since I don't want to re-work how Marshallers, etc 
get a hold of the class loader.  They use TCCLs right now, and while 
sub-optimal in many ways, this approach gives us an easy mechanism to implement 
while still preventing any leaks, etc otherwise common with TCCLs.

Cheers
Manik

--
Manik Surtani
ma...@jboss.org
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org




___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Jason T. Greene]

On 5/19/11 5:11 AM, Manik Surtani wrote:
> Guys - what are we talking about?  Specifying ClassLoaders is only
> meaningful if storeAsBinary is set to true.
>
> In general, any situation where you have code booted off different
> ClassLoaders running in the same JVM and sharing the same cache (or
> cache manager), you would *need* to set storeAsBinary to true to get
> around deserialization issues on remote nodes.
>
> StoreAsBinary = false only really works for trivial cases where
> caches/cache managers run in environments where only one cache loader
> is in effect.  I.e., *not* JBoss/Java EE/Hibernate/OSGi/etc.  This is
> one of the reasons why we considered setting storeAsBinary to true by
> default (and we see similar techniques in competing data grids).

With AS7 we actually have the ability now that you could actually locate 
the proper module/deployment that had the classes to deserialize, and 
you could even wait on the corresponding service for them to become 
available. OSGi also has a similar ability.

The problem though is that the deserialization thread would have to 
potentially block, so you would have to do something like have a tiny 
timeout and fallback to the storeAsBinary=true approach if the module 
isnt available yet.

Even worse is if someone constructed their own classloader, or used a 
framework that defined its own classloading, those types would not be 
accessible for the same reasons as in the past.

Lastly, it would require more work to implement since it would have to 
be tailored for each environment (OSGI and AS7 and so on).

So I think to ever justify it, it would take some very convincing 
numbers that eager deserialization is indeed better.

-- 
Jason T. Greene
JBoss, a division of Red Hat
___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Jason T. Greene]

On 5/18/11 11:06 AM, Manik Surtani wrote:

>
> 1) Class loader per session/cache.
>
> I like Jason/Sanne/Trustin's suggestions of a session-like contract, and 
> specifically I think this is best achieved as a delegate to a cache, again as 
> suggested elsewhere by Pete, etc.  E.g.,
>
>   Cache  myCache = cacheManager.getCache("myCache", myClassLoader);

-snip-

I would recommend leaving this open to store other per-session 
configuration values (perhaps with a builder), and some of them mutable.
This will allow you to completely eliminate the ThreadLocal context 
stuff used today which is both faster and more robust (the gc will clean 
up the state for you).

> 2) Class loader per invocation.
>

If this "session" notion has some mutable values, you could also make CL 
mutable:

cacheSession.setClassLoader(blah);
cacheSession.setFlags(FORCE_WRITE_LOCK)

cacheSession.put/get
repeat

>
> 3) Can all OSGi requirements be handled by (1)? I would guess so, from what I 
> have read here, since the class loader is explicitly passed in when getting a 
> handle on a cache.

Definitely.

-- 
Jason T. Greene
JBoss, a division of Red Hat
___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Dan Berindei]

On Thu, May 19, 2011 at 1:31 PM, Galder Zamarreño  wrote:
>
> On May 19, 2011, at 12:11 PM, Manik Surtani wrote:
>
>> Guys - what are we talking about?  Specifying ClassLoaders is only 
>> meaningful if storeAsBinary is set to true.
>
> Ok, that was not clear to me throughout the discussion.
>

That wasn't clear for me either. And we have at least one user trying
storeAsBinary==true because it has much better performance
(http://community.jboss.org/message/604831). Maybe the performance
difference isn't so great any more after the latest commits, but I'm
sure there is still a difference.

I'm not suggesting that we absolutely need to make storeAsBinary==true
work with multiple classloaders per cache or even per cache manager,
but we should support an alternative for OSGi users who want to use
storeAsBinary==true. That means at least having a clear way to specify
in one classloader per cache manager.

Note that this will also help us in the storeAsBinary==false case,
because then we will be able to set the bootstrap classloader as the
cache manager classloader and so will be able to avoid the cache
manager threads hanging on to an undeployed application's classes.


>> In general, any situation where you have code booted off different 
>> ClassLoaders running in the same JVM and sharing the same cache (or cache 
>> manager), you would *need* to set storeAsBinary to true to get around 
>> deserialization issues on remote nodes.
>>
>> StoreAsBinary = false only really works for trivial cases where caches/cache 
>> managers run in environments where only one cache loader is in effect.  
>> I.e., *not* JBoss/Java EE/Hibernate/OSGi/etc.  This is one of the reasons 
>> why we considered setting storeAsBinary to true by default (and we see 
>> similar techniques in competing data grids).
>

We already have an example of a guy who managed to make it work, we
just need to specify the behaviour of Infinispan so that he can rely
on it working.

Cheers
Dan


> This is clear now, thanks.
>
>>
>> Cheers
>> Manik
>>
>>
>> On 19 May 2011, at 10:55, Galder Zamarreño wrote:
>>
>>> would be different cache instances. The problem then is that if an RPC 
>>> comes for "entities" cache and entity P1, which of the "entities" caches do 
>>> I go for? You'd need to know which classloader P1 is living in the remote 
>>> node and you'd have to now that at the Infinispan level to be able to store 
>>> it in a non-binary format.
>>
>> --
>> Manik Surtani
>> ma...@jboss.org
>> twitter.com/maniksurtani
>>
>> Lead, Infinispan
>> http://www.infinispan.org
>>
>>
>>
>>
>> ___
>> infinispan-dev mailing list
>> infinispan-dev@lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
> --
> Galder Zamarreño
> Sr. Software Engineer
> Infinispan, JBoss Cache
>
>
> ___
> infinispan-dev mailing list
> infinispan-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>

___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Galder Zamarreño]

On May 19, 2011, at 12:11 PM, Manik Surtani wrote:

> Guys - what are we talking about?  Specifying ClassLoaders is only meaningful 
> if storeAsBinary is set to true.

Ok, that was not clear to me throughout the discussion.

> In general, any situation where you have code booted off different 
> ClassLoaders running in the same JVM and sharing the same cache (or cache 
> manager), you would *need* to set storeAsBinary to true to get around 
> deserialization issues on remote nodes.
> 
> StoreAsBinary = false only really works for trivial cases where caches/cache 
> managers run in environments where only one cache loader is in effect.  I.e., 
> *not* JBoss/Java EE/Hibernate/OSGi/etc.  This is one of the reasons why we 
> considered setting storeAsBinary to true by default (and we see similar 
> techniques in competing data grids).

This is clear now, thanks.

> 
> Cheers
> Manik
> 
> 
> On 19 May 2011, at 10:55, Galder Zamarreño wrote:
> 
>> would be different cache instances. The problem then is that if an RPC comes 
>> for "entities" cache and entity P1, which of the "entities" caches do I go 
>> for? You'd need to know which classloader P1 is living in the remote node 
>> and you'd have to now that at the Infinispan level to be able to store it in 
>> a non-binary format.
> 
> --
> Manik Surtani
> ma...@jboss.org
> twitter.com/maniksurtani
> 
> Lead, Infinispan
> http://www.infinispan.org
> 
> 
> 
> 
> ___
> infinispan-dev mailing list
> infinispan-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Galder Zamarreño]

On May 19, 2011, at 12:09 PM, Sanne Grinovero wrote:

>>> > 
>> How do you do that? Apps don't have any direct access to the Hibernate Cache.
>> 
>>> I assume the keys being used in the 2LC use entity names or class
>>> names combined with the primary keys;
>>> it's totally possible to have multiple applications using the same
>>> entity names, which is quite common in my experience.
>> 
>> That's what region name comes in. Multiple apps shouldn't have the same 
>> region name. That's what discriminates entities in different apps.
> 
> ah, thank you I finally understand why the configuration property
> hibernate.cache.region_prefix
> can be very useful.
> Actually, it would be nice to document this explicitly as otherwise
> people might end up deserializing a binary stream in the wrong
> application..

On the contrary, the thing is that in multi-app environments, i.e. JBoss AS, 
people shouldn't fiddle with this cos IIRC, the JPA/Hibernate deployer makes 
sure that the region prefix has the correct information about the deployment.

So, better not touch it and let the integration layer do what's best at, 
integrate multiple apps in a single environment.

Cheers,
--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Manik Surtani]

Guys - what are we talking about?  Specifying ClassLoaders is only meaningful 
if storeAsBinary is set to true.

In general, any situation where you have code booted off different ClassLoaders 
running in the same JVM and sharing the same cache (or cache manager), you 
would *need* to set storeAsBinary to true to get around deserialization issues 
on remote nodes.

StoreAsBinary = false only really works for trivial cases where caches/cache 
managers run in environments where only one cache loader is in effect.  I.e., 
*not* JBoss/Java EE/Hibernate/OSGi/etc.  This is one of the reasons why we 
considered setting storeAsBinary to true by default (and we see similar 
techniques in competing data grids).

Cheers
Manik


On 19 May 2011, at 10:55, Galder Zamarreño wrote:

> would be different cache instances. The problem then is that if an RPC comes 
> for "entities" cache and entity P1, which of the "entities" caches do I go 
> for? You'd need to know which classloader P1 is living in the remote node and 
> you'd have to now that at the Infinispan level to be able to store it in a 
> non-binary format.

--
Manik Surtani
ma...@jboss.org
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org




___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Sanne Grinovero]

2011/5/19 Galder Zamarreño :
>
> On May 19, 2011, at 10:04 AM, Sanne Grinovero wrote:
>
>> 2011/5/19 Dan Berindei :
>>> On Wed, May 18, 2011 at 7:06 PM, Manik Surtani  wrote:
 Hi guys

 Sorry I've been absent from this thread for a while now (it's been growing 
 faster than I've been able to deal with email backlog!)

 Anyway, this is a very interesting discussion.  To summarise - as Pete did 
 at some point - there are 2 goals here:

 1.  Safe and intuitive use of an appropriate classloader
 2.  Safe type system for return values.

 I think the far more pressing concern is (1) so I'd like to focus on that. 
  If we think (2) is pressing enough a concern, we should spawn a separate 
 thread and discuss there.

 So, onto the issue of safe classloading.

 1) Class loader per session/cache.

 I like Jason/Sanne/Trustin's suggestions of a session-like contract, and 
 specifically I think this is best achieved as a delegate to a cache, again 
 as suggested elsewhere by Pete, etc.  E.g.,

        Cache myCache = cacheManager.getCache("myCache", 
 myClassLoader);

 and what is returned is something that delegates to the actual cache, 
 making sure the TCCL is set and re-set appropriately.  The handle to the 
 cache is effectively your "session" and each webapp, etc in an EE 
 environment will have its own handle.  I propose using the TCCL as an 
 internal implementation detail within this delegate, helps with making 
 sure it is carefully managed and cleaned up while not re-engineering loads 
 of internals.

>>>
>>> I like the API but I would not recommend using the TCCL for this. I
>>> was able to get a nice perf jump in the HotRod client by skipping 2
>>> Thread.setContextClassLoader() calls on each cache operation (1 to set
>>> the TCCL we wanted and 1 to restore the original TCCL). Setting the
>>> TCCL is a privileged operation, so it has to go through a
>>> SecurityManager and that is very slow.
>>>
>>>
 I think EmbeddedCacheManager.getCache(String name, ClassLoader cl) is 
 enough ... it is clear enough, and I don't see the need for overloaded 
 getCache(name, classOfWhichClassLoaderIWishToUse).

>>>
>>> I agree, a Cache.usingClassloader(classOfWhichClassLoaderIWishToUse)
>>> overload would have made sense because the method name already
>>> communicates the intention, but a getCache(name, clazz) overload is
>>> too obscure.
>>>
>>>
 2) Class loader per invocation.

 I've been less than happy with this, not just because it pollutes the API 
 but that it adds a layer of confusion.  If all use cases discussed can be 
 solved with (1) above, then I'd prefer to just do that.

 The way I see it, most user apps directly using Infinispan would only be 
 exposed to a single class loader per cache reference (even if multiple 
 references talk to the same cache).

 Frameworks, OTOH, are a bit tougher, Hibernate being a good example on 
 this thread.  So this is a question for Galder - is it feasible to 
 maintain several references to a cache, one for each app/persistence unit?

 3) Can all OSGi requirements be handled by (1)? I would guess so, from 
 what I have read here, since the class loader is explicitly passed in when 
 getting a handle on a cache.

>>>
>>> Yes, the only difference is that OSGi goesn't make any guarantees
>>> about the TCCL, so passing the classloader explicitly will work in all
>>> environments. However,
>>>
>>> 4) What about storeAsBinary="false"? Threads processing requests from
>>> other nodes are not associated with any CacheManager.getCache(name,
>>> classloader) call, and they also have to unmarshall values with this
>>> setting.
>>>
>>> Since Hibernate already mandates storeAsBinary="true" for its 2LC, we
>>> can probably get away with only supporting one classloader per cache
>>> in the storeAsBinary="false" case.
>>>
>>> Still, we can't rely on the TCCL of the background threads because
>>> those threads are shared between all the caches in a CacheManager. In
>>> fact we should probably set the TCCL to null for all background
>>> threads created by Infinispan, or we risk keeping the classes of the
>>> first application/bundle that called us alive as long as those threads
>>> are still running.
>>>
>>> Dan
>>
>> Totally agree with all you said, great analysis!
>>
>> So it looks like that all Caches being shared across different
>> classloaders (deployed applications)
>> should be used only with storeAsBinary="false", still I'm having some
>> doubts about how even that is safe.
>> How can we prevent two applications both using hibernate from mixing
>> the cache keys and values?
>
> How do you do that? Apps don't have any direct access to the Hibernate Cache.
>
>> I assume the keys being used in the 2LC use entity names or class
>> names combined with the primary key

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

[Galder Zamarreño]

On May 19, 2011, at 10:04 AM, Sanne Grinovero wrote:

> 2011/5/19 Dan Berindei :
>> On Wed, May 18, 2011 at 7:06 PM, Manik Surtani  wrote:
>>> Hi guys
>>> 
>>> Sorry I've been absent from this thread for a while now (it's been growing 
>>> faster than I've been able to deal with email backlog!)
>>> 
>>> Anyway, this is a very interesting discussion.  To summarise - as Pete did 
>>> at some point - there are 2 goals here:
>>> 
>>> 1.  Safe and intuitive use of an appropriate classloader
>>> 2.  Safe type system for return values.
>>> 
>>> I think the far more pressing concern is (1) so I'd like to focus on that.  
>>> If we think (2) is pressing enough a concern, we should spawn a separate 
>>> thread and discuss there.
>>> 
>>> So, onto the issue of safe classloading.
>>> 
>>> 1) Class loader per session/cache.
>>> 
>>> I like Jason/Sanne/Trustin's suggestions of a session-like contract, and 
>>> specifically I think this is best achieved as a delegate to a cache, again 
>>> as suggested elsewhere by Pete, etc.  E.g.,
>>> 
>>>Cache myCache = cacheManager.getCache("myCache", 
>>> myClassLoader);
>>> 
>>> and what is returned is something that delegates to the actual cache, 
>>> making sure the TCCL is set and re-set appropriately.  The handle to the 
>>> cache is effectively your "session" and each webapp, etc in an EE 
>>> environment will have its own handle.  I propose using the TCCL as an 
>>> internal implementation detail within this delegate, helps with making sure 
>>> it is carefully managed and cleaned up while not re-engineering loads of 
>>> internals.
>>> 
>> 
>> I like the API but I would not recommend using the TCCL for this. I
>> was able to get a nice perf jump in the HotRod client by skipping 2
>> Thread.setContextClassLoader() calls on each cache operation (1 to set
>> the TCCL we wanted and 1 to restore the original TCCL). Setting the
>> TCCL is a privileged operation, so it has to go through a
>> SecurityManager and that is very slow.
>> 
>> 
>>> I think EmbeddedCacheManager.getCache(String name, ClassLoader cl) is 
>>> enough ... it is clear enough, and I don't see the need for overloaded 
>>> getCache(name, classOfWhichClassLoaderIWishToUse).
>>> 
>> 
>> I agree, a Cache.usingClassloader(classOfWhichClassLoaderIWishToUse)
>> overload would have made sense because the method name already
>> communicates the intention, but a getCache(name, clazz) overload is
>> too obscure.
>> 
>> 
>>> 2) Class loader per invocation.
>>> 
>>> I've been less than happy with this, not just because it pollutes the API 
>>> but that it adds a layer of confusion.  If all use cases discussed can be 
>>> solved with (1) above, then I'd prefer to just do that.
>>> 
>>> The way I see it, most user apps directly using Infinispan would only be 
>>> exposed to a single class loader per cache reference (even if multiple 
>>> references talk to the same cache).
>>> 
>>> Frameworks, OTOH, are a bit tougher, Hibernate being a good example on this 
>>> thread.  So this is a question for Galder - is it feasible to maintain 
>>> several references to a cache, one for each app/persistence unit?
>>> 
>>> 3) Can all OSGi requirements be handled by (1)? I would guess so, from what 
>>> I have read here, since the class loader is explicitly passed in when 
>>> getting a handle on a cache.
>>> 
>> 
>> Yes, the only difference is that OSGi goesn't make any guarantees
>> about the TCCL, so passing the classloader explicitly will work in all
>> environments. However,
>> 
>> 4) What about storeAsBinary="false"? Threads processing requests from
>> other nodes are not associated with any CacheManager.getCache(name,
>> classloader) call, and they also have to unmarshall values with this
>> setting.
>> 
>> Since Hibernate already mandates storeAsBinary="true" for its 2LC, we
>> can probably get away with only supporting one classloader per cache
>> in the storeAsBinary="false" case.
>> 
>> Still, we can't rely on the TCCL of the background threads because
>> those threads are shared between all the caches in a CacheManager. In
>> fact we should probably set the TCCL to null for all background
>> threads created by Infinispan, or we risk keeping the classes of the
>> first application/bundle that called us alive as long as those threads
>> are still running.
>> 
>> Dan
> 
> Totally agree with all you said, great analysis!
> 
> So it looks like that all Caches being shared across different
> classloaders (deployed applications)
> should be used only with storeAsBinary="false", still I'm having some
> doubts about how even that is safe.
> How can we prevent two applications both using hibernate from mixing
> the cache keys and values?

How do you do that? Apps don't have any direct access to the Hibernate Cache.

> I assume the keys being used in the 2LC use entity names or class
> names combined with the primary keys;
> it's totally possible to have multiple applications using the same
> entity names, which is quite common in my

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

[Galder Zamarreño]

On May 19, 2011, at 11:55 AM, Galder Zamarreño wrote:

> 
> On May 19, 2011, at 11:45 AM, Galder Zamarreño wrote:
> 
>> 
>> On May 18, 2011, at 6:06 PM, Manik Surtani wrote:
>> 
>>> 2) Class loader per invocation.
>>> 
>>> I've been less than happy with this, not just because it pollutes the API 
>>> but that it adds a layer of confusion.  If all use cases discussed can be 
>>> solved with (1) above, then I'd prefer to just do that.
>>> 
>>> The way I see it, most user apps directly using Infinispan would only be 
>>> exposed to a single class loader per cache reference (even if multiple 
>>> references talk to the same cache).  
>>> 
>>> Frameworks, OTOH, are a bit tougher, Hibernate being a good example on this 
>>> thread.  So this is a question for Galder - is it feasible to maintain 
>>> several references to a cache, one for each app/persistence unit?
>> 
>> Sorry, missed the second part of the email, forget it my questions are 
>> irrelevant after this. 
>> 
>> I think this might be doable. Each entity/collection region has a cache 
>> associated which is built of the region factory. As long as when the 
>> region's classloader can be identified at that point and passed to the cache 
>> manager on cache retrieval, we should be Ok.
> 
> Actually Manik, for this to really work, a Cache would need to be identified 
> not only by its name but by its classloader too, so:
> 
> cacheManager.getCache("entities", CL1)
> and
> cacheManager.getCache("entities", CL2)
> 
> would be different cache instances. The problem then is that if an RPC comes 
> for "entities" cache and entity P1, which of the "entities" caches do I go 
> for? You'd need to know which classloader P1 is living in the remote node and 
> you'd have to now that at the Infinispan level to be able to store it in a 
> non-binary format.

And not only you have the problem of how to know the classloader associated 
with P1, you also have to remember that the node to which P1 is replicated to 
might not have P1 deployed. You're back to the asymmetric cluster issue where 
the entity might not be deployed in the remote node ( == the cache you're 
talking to might not be deployed in that node).

When 2LC uses a common cache with storeAsBinary=true, we won't have this 
problem cos the entity can be replicated to any node and remain in binary 
format until the app is deployed and P1 is requested.

--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Galder Zamarreño]

On May 19, 2011, at 11:45 AM, Galder Zamarreño wrote:

> 
> On May 18, 2011, at 6:06 PM, Manik Surtani wrote:
> 
>> 2) Class loader per invocation.
>> 
>> I've been less than happy with this, not just because it pollutes the API 
>> but that it adds a layer of confusion.  If all use cases discussed can be 
>> solved with (1) above, then I'd prefer to just do that.
>> 
>> The way I see it, most user apps directly using Infinispan would only be 
>> exposed to a single class loader per cache reference (even if multiple 
>> references talk to the same cache).  
>> 
>> Frameworks, OTOH, are a bit tougher, Hibernate being a good example on this 
>> thread.  So this is a question for Galder - is it feasible to maintain 
>> several references to a cache, one for each app/persistence unit?
> 
> Sorry, missed the second part of the email, forget it my questions are 
> irrelevant after this. 
> 
> I think this might be doable. Each entity/collection region has a cache 
> associated which is built of the region factory. As long as when the region's 
> classloader can be identified at that point and passed to the cache manager 
> on cache retrieval, we should be Ok.

Actually Manik, for this to really work, a Cache would need to be identified 
not only by its name but by its classloader too, so:

cacheManager.getCache("entities", CL1)
and
cacheManager.getCache("entities", CL2)

would be different cache instances. The problem then is that if an RPC comes 
for "entities" cache and entity P1, which of the "entities" caches do I go for? 
You'd need to know which classloader P1 is living in the remote node and you'd 
have to now that at the Infinispan level to be able to store it in a non-binary 
format.

> 
> Otherwise, storeAsBinary would be the alternative for these cases.
> 
>> 
>> 3) Can all OSGi requirements be handled by (1)? I would guess so, from what 
>> I have read here, since the class loader is explicitly passed in when 
>> getting a handle on a cache.
>> 
>> Cheers
>> Manik
>> 
>> 
>> On 27 Apr 2011, at 17:39, Jason T. Greene wrote:
>> 
>>> Available here:
>>> https://github.com/infinispan/infinispan/pull/278
>>> 
>>> The problem is basically that infinispan currently is using TCCL for all 
>>> class loading and resource loading. This has a lot of problems in 
>>> modular containers (OSGi, AS7, etc), where you dont have framework 
>>> implementation classes on the same classloader as user classes (that is 
>>> how they achieve true isolation)
>>> 
>>> You can read about this in more detail here:
>>> http://community.jboss.org/wiki/ModuleCompatibleClassloadingGuide
>>> 
>>> The patch in the pull request is a first step, and should fix many 
>>> issues, but it does not address all (there is still a lot of TCCL usage 
>>> spread out among cacheloaders and so on), and ultimately it's just a 
>>> work around. It should, however, be compatible in any other non-modular 
>>> environment.
>>> 
>>> Really the ultimate solution is to setup a proper demarcation between 
>>> what the user is supposed to provide, and what is expected to be bundled 
>>> with infinispan. Whenever there is something the user can provide a 
>>> class, then the API should accept a classloader to load that class from. 
>>> If it's a class that is just internal wiring of infinispan, then 
>>> Infinispan's defining classloader should always be used.
>>> 
>>> The one case I can think of in infnispan where TCCL really makes sense 
>>> is in the case of lazy deserialization from an EE application. However 
>>> that is only guaranteed to work when you are executing in a context that 
>>> has that style of contract (like in an EE component). There are many 
>>> other cases though where someone would expect it to work from a non-EE 
>>> context (e.g. from a thread pool). What you really want is the caller's 
>>> classloader, but that is not cheap to get at, so it's something that 
>>> should be supplied as part of the API interaction (in the case where 
>>> there is no EE context). Alternatively you can just just require that 
>>> folks push/pop TCCL, but users often get this wrong.
>>> 
>>> Thanks!
>>> 
>>> -- 
>>> Jason T. Greene
>>> JBoss, a division of Red Hat
>>> ___
>>> infinispan-dev mailing list
>>> infinispan-dev@lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>> 
>> --
>> Manik Surtani
>> ma...@jboss.org
>> twitter.com/maniksurtani
>> 
>> Lead, Infinispan
>> http://www.infinispan.org
>> 
>> 
>> 
>> 
>> ___
>> infinispan-dev mailing list
>> infinispan-dev@lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
> 
> --
> Galder Zamarreño
> Sr. Software Engineer
> Infinispan, JBoss Cache
> 
> 
> ___
> infinispan-dev mailing list
> infinispan-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Galder Zamarreño
Sr. Software Engineer
Infinispan

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

[Galder Zamarreño]

On May 18, 2011, at 6:06 PM, Manik Surtani wrote:

> 2) Class loader per invocation.
> 
> I've been less than happy with this, not just because it pollutes the API but 
> that it adds a layer of confusion.  If all use cases discussed can be solved 
> with (1) above, then I'd prefer to just do that.
> 
> The way I see it, most user apps directly using Infinispan would only be 
> exposed to a single class loader per cache reference (even if multiple 
> references talk to the same cache).  
> 
> Frameworks, OTOH, are a bit tougher, Hibernate being a good example on this 
> thread.  So this is a question for Galder - is it feasible to maintain 
> several references to a cache, one for each app/persistence unit?

Sorry, missed the second part of the email, forget it my questions are 
irrelevant after this. 

I think this might be doable. Each entity/collection region has a cache 
associated which is built of the region factory. As long as when the region's 
classloader can be identified at that point and passed to the cache manager on 
cache retrieval, we should be Ok.

Otherwise, storeAsBinary would be the alternative for these cases.

> 
> 3) Can all OSGi requirements be handled by (1)? I would guess so, from what I 
> have read here, since the class loader is explicitly passed in when getting a 
> handle on a cache.
> 
> Cheers
> Manik
> 
> 
> On 27 Apr 2011, at 17:39, Jason T. Greene wrote:
> 
>> Available here:
>> https://github.com/infinispan/infinispan/pull/278
>> 
>> The problem is basically that infinispan currently is using TCCL for all 
>> class loading and resource loading. This has a lot of problems in 
>> modular containers (OSGi, AS7, etc), where you dont have framework 
>> implementation classes on the same classloader as user classes (that is 
>> how they achieve true isolation)
>> 
>> You can read about this in more detail here:
>> http://community.jboss.org/wiki/ModuleCompatibleClassloadingGuide
>> 
>> The patch in the pull request is a first step, and should fix many 
>> issues, but it does not address all (there is still a lot of TCCL usage 
>> spread out among cacheloaders and so on), and ultimately it's just a 
>> work around. It should, however, be compatible in any other non-modular 
>> environment.
>> 
>> Really the ultimate solution is to setup a proper demarcation between 
>> what the user is supposed to provide, and what is expected to be bundled 
>> with infinispan. Whenever there is something the user can provide a 
>> class, then the API should accept a classloader to load that class from. 
>> If it's a class that is just internal wiring of infinispan, then 
>> Infinispan's defining classloader should always be used.
>> 
>> The one case I can think of in infnispan where TCCL really makes sense 
>> is in the case of lazy deserialization from an EE application. However 
>> that is only guaranteed to work when you are executing in a context that 
>> has that style of contract (like in an EE component). There are many 
>> other cases though where someone would expect it to work from a non-EE 
>> context (e.g. from a thread pool). What you really want is the caller's 
>> classloader, but that is not cheap to get at, so it's something that 
>> should be supplied as part of the API interaction (in the case where 
>> there is no EE context). Alternatively you can just just require that 
>> folks push/pop TCCL, but users often get this wrong.
>> 
>> Thanks!
>> 
>> -- 
>> Jason T. Greene
>> JBoss, a division of Red Hat
>> ___
>> infinispan-dev mailing list
>> infinispan-dev@lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
> 
> --
> Manik Surtani
> ma...@jboss.org
> twitter.com/maniksurtani
> 
> Lead, Infinispan
> http://www.infinispan.org
> 
> 
> 
> 
> ___
> infinispan-dev mailing list
> infinispan-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Galder Zamarreño]

On May 18, 2011, at 6:06 PM, Manik Surtani wrote:

> Hi guys
> 
> Sorry I've been absent from this thread for a while now (it's been growing 
> faster than I've been able to deal with email backlog!)
> 
> Anyway, this is a very interesting discussion.  To summarise - as Pete did at 
> some point - there are 2 goals here:
> 
> 1.  Safe and intuitive use of an appropriate classloader
> 2.  Safe type system for return values.
> 
> I think the far more pressing concern is (1) so I'd like to focus on that.  
> If we think (2) is pressing enough a concern, we should spawn a separate 
> thread and discuss there.
> 
> So, onto the issue of safe classloading.
> 
> 1) Class loader per session/cache.
> 
> I like Jason/Sanne/Trustin's suggestions of a session-like contract, and 
> specifically I think this is best achieved as a delegate to a cache, again as 
> suggested elsewhere by Pete, etc.  E.g.,
> 
>   Cache myCache = cacheManager.getCache("myCache", myClassLoader);
> 
> and what is returned is something that delegates to the actual cache, making 
> sure the TCCL is set and re-set appropriately.  The handle to the cache is 
> effectively your "session" and each webapp, etc in an EE environment will 
> have its own handle.  I propose using the TCCL as an internal implementation 
> detail within this delegate, helps with making sure it is carefully managed 
> and cleaned up while not re-engineering loads of internals.
> 
> I think EmbeddedCacheManager.getCache(String name, ClassLoader cl) is enough 
> ... it is clear enough, and I don't see the need for overloaded 
> getCache(name, classOfWhichClassLoaderIWishToUse).

What about the unmarshalling part where a cache can be unmarshalled with 
several classloaders? Assuming that the classloader you pass here is just part 
of the handle, or the cache delegate, you still need storeAsBinary for 
unmarshalling part, correct?

If you expect only one classloader to interact with the cache, you could do 
away with storeAsBinary and get cache's associated classloader to deserialize 
it.

Correct?
--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Sanne Grinovero]

2011/5/19 Dan Berindei :
> On Wed, May 18, 2011 at 7:06 PM, Manik Surtani  wrote:
>> Hi guys
>>
>> Sorry I've been absent from this thread for a while now (it's been growing 
>> faster than I've been able to deal with email backlog!)
>>
>> Anyway, this is a very interesting discussion.  To summarise - as Pete did 
>> at some point - there are 2 goals here:
>>
>> 1.  Safe and intuitive use of an appropriate classloader
>> 2.  Safe type system for return values.
>>
>> I think the far more pressing concern is (1) so I'd like to focus on that.  
>> If we think (2) is pressing enough a concern, we should spawn a separate 
>> thread and discuss there.
>>
>> So, onto the issue of safe classloading.
>>
>> 1) Class loader per session/cache.
>>
>> I like Jason/Sanne/Trustin's suggestions of a session-like contract, and 
>> specifically I think this is best achieved as a delegate to a cache, again 
>> as suggested elsewhere by Pete, etc.  E.g.,
>>
>>        Cache myCache = cacheManager.getCache("myCache", myClassLoader);
>>
>> and what is returned is something that delegates to the actual cache, making 
>> sure the TCCL is set and re-set appropriately.  The handle to the cache is 
>> effectively your "session" and each webapp, etc in an EE environment will 
>> have its own handle.  I propose using the TCCL as an internal implementation 
>> detail within this delegate, helps with making sure it is carefully managed 
>> and cleaned up while not re-engineering loads of internals.
>>
>
> I like the API but I would not recommend using the TCCL for this. I
> was able to get a nice perf jump in the HotRod client by skipping 2
> Thread.setContextClassLoader() calls on each cache operation (1 to set
> the TCCL we wanted and 1 to restore the original TCCL). Setting the
> TCCL is a privileged operation, so it has to go through a
> SecurityManager and that is very slow.
>
>
>> I think EmbeddedCacheManager.getCache(String name, ClassLoader cl) is enough 
>> ... it is clear enough, and I don't see the need for overloaded 
>> getCache(name, classOfWhichClassLoaderIWishToUse).
>>
>
> I agree, a Cache.usingClassloader(classOfWhichClassLoaderIWishToUse)
> overload would have made sense because the method name already
> communicates the intention, but a getCache(name, clazz) overload is
> too obscure.
>
>
>> 2) Class loader per invocation.
>>
>> I've been less than happy with this, not just because it pollutes the API 
>> but that it adds a layer of confusion.  If all use cases discussed can be 
>> solved with (1) above, then I'd prefer to just do that.
>>
>> The way I see it, most user apps directly using Infinispan would only be 
>> exposed to a single class loader per cache reference (even if multiple 
>> references talk to the same cache).
>>
>> Frameworks, OTOH, are a bit tougher, Hibernate being a good example on this 
>> thread.  So this is a question for Galder - is it feasible to maintain 
>> several references to a cache, one for each app/persistence unit?
>>
>> 3) Can all OSGi requirements be handled by (1)? I would guess so, from what 
>> I have read here, since the class loader is explicitly passed in when 
>> getting a handle on a cache.
>>
>
> Yes, the only difference is that OSGi goesn't make any guarantees
> about the TCCL, so passing the classloader explicitly will work in all
> environments. However,
>
> 4) What about storeAsBinary="false"? Threads processing requests from
> other nodes are not associated with any CacheManager.getCache(name,
> classloader) call, and they also have to unmarshall values with this
> setting.
>
> Since Hibernate already mandates storeAsBinary="true" for its 2LC, we
> can probably get away with only supporting one classloader per cache
> in the storeAsBinary="false" case.
>
> Still, we can't rely on the TCCL of the background threads because
> those threads are shared between all the caches in a CacheManager. In
> fact we should probably set the TCCL to null for all background
> threads created by Infinispan, or we risk keeping the classes of the
> first application/bundle that called us alive as long as those threads
> are still running.
>
> Dan

Totally agree with all you said, great analysis!

So it looks like that all Caches being shared across different
classloaders (deployed applications)
should be used only with storeAsBinary="false", still I'm having some
doubts about how even that is safe.
How can we prevent two applications both using hibernate from mixing
the cache keys and values?
I assume the keys being used in the 2LC use entity names or class
names combined with the primary keys;
it's totally possible to have multiple applications using the same
entity names, which is quite common in my experience.

The configurations defined in the app server could be used as
templates to start/stop specific caches for each deployment,
generating a unique name depending on the ear/war/.. in such a way to
be consistent with names on other nodes.
Was this all designed as a workaround for the

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

[Dan Berindei]

On Wed, May 18, 2011 at 7:06 PM, Manik Surtani  wrote:
> Hi guys
>
> Sorry I've been absent from this thread for a while now (it's been growing 
> faster than I've been able to deal with email backlog!)
>
> Anyway, this is a very interesting discussion.  To summarise - as Pete did at 
> some point - there are 2 goals here:
>
> 1.  Safe and intuitive use of an appropriate classloader
> 2.  Safe type system for return values.
>
> I think the far more pressing concern is (1) so I'd like to focus on that.  
> If we think (2) is pressing enough a concern, we should spawn a separate 
> thread and discuss there.
>
> So, onto the issue of safe classloading.
>
> 1) Class loader per session/cache.
>
> I like Jason/Sanne/Trustin's suggestions of a session-like contract, and 
> specifically I think this is best achieved as a delegate to a cache, again as 
> suggested elsewhere by Pete, etc.  E.g.,
>
>        Cache myCache = cacheManager.getCache("myCache", myClassLoader);
>
> and what is returned is something that delegates to the actual cache, making 
> sure the TCCL is set and re-set appropriately.  The handle to the cache is 
> effectively your "session" and each webapp, etc in an EE environment will 
> have its own handle.  I propose using the TCCL as an internal implementation 
> detail within this delegate, helps with making sure it is carefully managed 
> and cleaned up while not re-engineering loads of internals.
>

I like the API but I would not recommend using the TCCL for this. I
was able to get a nice perf jump in the HotRod client by skipping 2
Thread.setContextClassLoader() calls on each cache operation (1 to set
the TCCL we wanted and 1 to restore the original TCCL). Setting the
TCCL is a privileged operation, so it has to go through a
SecurityManager and that is very slow.


> I think EmbeddedCacheManager.getCache(String name, ClassLoader cl) is enough 
> ... it is clear enough, and I don't see the need for overloaded 
> getCache(name, classOfWhichClassLoaderIWishToUse).
>

I agree, a Cache.usingClassloader(classOfWhichClassLoaderIWishToUse)
overload would have made sense because the method name already
communicates the intention, but a getCache(name, clazz) overload is
too obscure.


> 2) Class loader per invocation.
>
> I've been less than happy with this, not just because it pollutes the API but 
> that it adds a layer of confusion.  If all use cases discussed can be solved 
> with (1) above, then I'd prefer to just do that.
>
> The way I see it, most user apps directly using Infinispan would only be 
> exposed to a single class loader per cache reference (even if multiple 
> references talk to the same cache).
>
> Frameworks, OTOH, are a bit tougher, Hibernate being a good example on this 
> thread.  So this is a question for Galder - is it feasible to maintain 
> several references to a cache, one for each app/persistence unit?
>
> 3) Can all OSGi requirements be handled by (1)? I would guess so, from what I 
> have read here, since the class loader is explicitly passed in when getting a 
> handle on a cache.
>

Yes, the only difference is that OSGi goesn't make any guarantees
about the TCCL, so passing the classloader explicitly will work in all
environments. However,

4) What about storeAsBinary="false"? Threads processing requests from
other nodes are not associated with any CacheManager.getCache(name,
classloader) call, and they also have to unmarshall values with this
setting.

Since Hibernate already mandates storeAsBinary="true" for its 2LC, we
can probably get away with only supporting one classloader per cache
in the storeAsBinary="false" case.

Still, we can't rely on the TCCL of the background threads because
those threads are shared between all the caches in a CacheManager. In
fact we should probably set the TCCL to null for all background
threads created by Infinispan, or we risk keeping the classes of the
first application/bundle that called us alive as long as those threads
are still running.

Dan


> Cheers
> Manik
>
>
> On 27 Apr 2011, at 17:39, Jason T. Greene wrote:
>
>> Available here:
>> https://github.com/infinispan/infinispan/pull/278
>>
>> The problem is basically that infinispan currently is using TCCL for all
>> class loading and resource loading. This has a lot of problems in
>> modular containers (OSGi, AS7, etc), where you dont have framework
>> implementation classes on the same classloader as user classes (that is
>> how they achieve true isolation)
>>
>> You can read about this in more detail here:
>> http://community.jboss.org/wiki/ModuleCompatibleClassloadingGuide
>>
>> The patch in the pull request is a first step, and should fix many
>> issues, but it does not address all (there is still a lot of TCCL usage
>> spread out among cacheloaders and so on), and ultimately it's just a
>> work around. It should, however, be compatible in any other non-modular
>> environment.
>>
>> Really the ultimate solution is to setup a proper demarcation between
>> what the user is 

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

[Manik Surtani]

Hi guys

Sorry I've been absent from this thread for a while now (it's been growing 
faster than I've been able to deal with email backlog!)

Anyway, this is a very interesting discussion.  To summarise - as Pete did at 
some point - there are 2 goals here:

1.  Safe and intuitive use of an appropriate classloader
2.  Safe type system for return values.

I think the far more pressing concern is (1) so I'd like to focus on that.  If 
we think (2) is pressing enough a concern, we should spawn a separate thread 
and discuss there.

So, onto the issue of safe classloading.

1) Class loader per session/cache.

I like Jason/Sanne/Trustin's suggestions of a session-like contract, and 
specifically I think this is best achieved as a delegate to a cache, again as 
suggested elsewhere by Pete, etc.  E.g.,

Cache myCache = cacheManager.getCache("myCache", myClassLoader);

and what is returned is something that delegates to the actual cache, making 
sure the TCCL is set and re-set appropriately.  The handle to the cache is 
effectively your "session" and each webapp, etc in an EE environment will have 
its own handle.  I propose using the TCCL as an internal implementation detail 
within this delegate, helps with making sure it is carefully managed and 
cleaned up while not re-engineering loads of internals.

I think EmbeddedCacheManager.getCache(String name, ClassLoader cl) is enough 
... it is clear enough, and I don't see the need for overloaded getCache(name, 
classOfWhichClassLoaderIWishToUse).

2) Class loader per invocation.

I've been less than happy with this, not just because it pollutes the API but 
that it adds a layer of confusion.  If all use cases discussed can be solved 
with (1) above, then I'd prefer to just do that.

The way I see it, most user apps directly using Infinispan would only be 
exposed to a single class loader per cache reference (even if multiple 
references talk to the same cache).  

Frameworks, OTOH, are a bit tougher, Hibernate being a good example on this 
thread.  So this is a question for Galder - is it feasible to maintain several 
references to a cache, one for each app/persistence unit?

3) Can all OSGi requirements be handled by (1)? I would guess so, from what I 
have read here, since the class loader is explicitly passed in when getting a 
handle on a cache.

Cheers
Manik


On 27 Apr 2011, at 17:39, Jason T. Greene wrote:

> Available here:
> https://github.com/infinispan/infinispan/pull/278
> 
> The problem is basically that infinispan currently is using TCCL for all 
> class loading and resource loading. This has a lot of problems in 
> modular containers (OSGi, AS7, etc), where you dont have framework 
> implementation classes on the same classloader as user classes (that is 
> how they achieve true isolation)
> 
> You can read about this in more detail here:
> http://community.jboss.org/wiki/ModuleCompatibleClassloadingGuide
> 
> The patch in the pull request is a first step, and should fix many 
> issues, but it does not address all (there is still a lot of TCCL usage 
> spread out among cacheloaders and so on), and ultimately it's just a 
> work around. It should, however, be compatible in any other non-modular 
> environment.
> 
> Really the ultimate solution is to setup a proper demarcation between 
> what the user is supposed to provide, and what is expected to be bundled 
> with infinispan. Whenever there is something the user can provide a 
> class, then the API should accept a classloader to load that class from. 
> If it's a class that is just internal wiring of infinispan, then 
> Infinispan's defining classloader should always be used.
> 
> The one case I can think of in infnispan where TCCL really makes sense 
> is in the case of lazy deserialization from an EE application. However 
> that is only guaranteed to work when you are executing in a context that 
> has that style of contract (like in an EE component). There are many 
> other cases though where someone would expect it to work from a non-EE 
> context (e.g. from a thread pool). What you really want is the caller's 
> classloader, but that is not cheap to get at, so it's something that 
> should be supplied as part of the API interaction (in the case where 
> there is no EE context). Alternatively you can just just require that 
> folks push/pop TCCL, but users often get this wrong.
> 
> Thanks!
> 
> -- 
> Jason T. Greene
> JBoss, a division of Red Hat
> ___
> infinispan-dev mailing list
> infinispan-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Manik Surtani
ma...@jboss.org
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org




___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Manik Surtani]

On 4 May 2011, at 14:55, Dan Berindei wrote:

> 
> If V is Set then Set.class.getClassLoader() will
> give you the system classloader, which in most cases won't be able the
> MyObject class. It may not even be a Set of course, it could
> be just Set.
> 
> We could have a "shortcut" so the users can pass in a class instead of
> a classloader to avoid writing ".getClassLoader()", but we shouldn't
> require any relation between the class passed in and the class of the
> return value.

Very good point.

--
Manik Surtani
ma...@jboss.org
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org



___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Galder Zamarreño]

On May 17, 2011, at 12:04 AM, Dan Berindei wrote:

> Not to be the guy that breaks the circle, how about the threads that
> handle incoming RPCs from other nodes? With storeAsBinary="false"
> those threads have to unmarshal the objects in order to store them on
> the local node, so we'd need a way to register a classloader with the
> cache, not just with the cache session.
> 
> If we use a single cache for all Hibernate entities, regardless of
> application, this means the cache could receive application objects
> from the other nodes without the application even being deployed. Does
> this mean we'll require storeAsBinary="true" in all Hibernate 2LC
> configurations?

storeAsBinary has always been a requirement for Hibernate 2LC and this use case 
is configured with it.

> 
> Dan
> 
> 
> On Mon, May 16, 2011 at 9:54 PM, David M. Lloyd  
> wrote:
>> I know, we can just attach the class loader to the cache!
>> 
>> Okay, just kidding, but Galder is right, this conversation is going in
>> circles.  We already discussed that in this thread and a number of
>> points were raised for and against.
>> 
>> On 05/16/2011 01:20 PM, Adrian Cole wrote:
>>> What about a helper that just returns a cache with a specific
>>> classloader from a cache?
>>> 
>>> cache.withLoader(cl).get(K key)
>>> 
>>> -a
>>> 
>>> 
>>> On Mon, May 16, 2011 at 11:14 AM, Galder Zamarreño >> > wrote:
>>> 
>>> 
>>> On May 16, 2011, at 7:57 PM, Sanne Grinovero wrote:
>>> 
>>>  > I don't like the TCCL either, so I'll repeat my suggestion from two
>>>  > weeks ago to just have:
>>>  >
>>>  > Cache c = cacheManager.getCache( cacheName, classLoader );
>>>  >
>>>  > sounds reasonable to me to have the application declare it's
>>> intentions once ?
>>>  >
>>>  > BTW I don't like
>>>  >
>>>  > "cache.get(K key, Class clazz)"
>>>  >
>>>  > as we're not speaking only about the get(K) method, but about many
>>>  > methods and this will explode the number of method of Cache; on the
>>>  > other hand I think it;s acceptable to have a single Cache instance
>>>  > used by a single application/classloader. You can still have multiple
>>>  > applications share the same underlying cache and use different
>>>  > classloaders:
>>> 
>>> Guys, we're going around in circles. As I said the other week, you
>>> can't assume 1 cache = 1 classloader cos for example in the
>>> Hibernate 2LC all entities will be stored in a single cache as
>>> opposed to today where we have a cache per entity. And if all
>>> entities are stored in the same cache, we potentially have a cache
>>> that contains data belonging to multiple cache loaders. And the
>>> reason for all this is cos we don't support asymmetric clusters.
>>> 
>>> Could someone start a design wiki to grab all the requirements?
>>> 
>>>  >
>>>  > getCache( cacheName, classLoader ) would return a delegate to the
>>>  > original cache, having a specific marshaller in the invocation
>>> context
>>>  > as Trustin was suggesting.
>>>  >
>>>  > Cheers,
>>>  > Sanne
>>>  >
>>>  >
>>>  > 2011/5/16 Pete Muir mailto:pm...@redhat.com>>:
>>>  >>
>>>  >> 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
>>> mailto:da...@redhat.com>> wrote:
>>>  > On 11/05/2011 17:54, Dan Berindei wrote:
>>>  >> On Wed, May 11, 2011 at 7:08 PM, Pete Muir>> >  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
>>>

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

[Dan Berindei]

Not to be the guy that breaks the circle, how about the threads that
handle incoming RPCs from other nodes? With storeAsBinary="false"
those threads have to unmarshal the objects in order to store them on
the local node, so we'd need a way to register a classloader with the
cache, not just with the cache session.

If we use a single cache for all Hibernate entities, regardless of
application, this means the cache could receive application objects
from the other nodes without the application even being deployed. Does
this mean we'll require storeAsBinary="true" in all Hibernate 2LC
configurations?

Dan


On Mon, May 16, 2011 at 9:54 PM, David M. Lloyd  wrote:
> I know, we can just attach the class loader to the cache!
>
> Okay, just kidding, but Galder is right, this conversation is going in
> circles.  We already discussed that in this thread and a number of
> points were raised for and against.
>
> On 05/16/2011 01:20 PM, Adrian Cole wrote:
>> What about a helper that just returns a cache with a specific
>> classloader from a cache?
>>
>> cache.withLoader(cl).get(K key)
>>
>> -a
>>
>>
>> On Mon, May 16, 2011 at 11:14 AM, Galder Zamarreño > > wrote:
>>
>>
>>     On May 16, 2011, at 7:57 PM, Sanne Grinovero wrote:
>>
>>      > I don't like the TCCL either, so I'll repeat my suggestion from two
>>      > weeks ago to just have:
>>      >
>>      > Cache c = cacheManager.getCache( cacheName, classLoader );
>>      >
>>      > sounds reasonable to me to have the application declare it's
>>     intentions once ?
>>      >
>>      > BTW I don't like
>>      >
>>      > "cache.get(K key, Class clazz)"
>>      >
>>      > as we're not speaking only about the get(K) method, but about many
>>      > methods and this will explode the number of method of Cache; on the
>>      > other hand I think it;s acceptable to have a single Cache instance
>>      > used by a single application/classloader. You can still have multiple
>>      > applications share the same underlying cache and use different
>>      > classloaders:
>>
>>     Guys, we're going around in circles. As I said the other week, you
>>     can't assume 1 cache = 1 classloader cos for example in the
>>     Hibernate 2LC all entities will be stored in a single cache as
>>     opposed to today where we have a cache per entity. And if all
>>     entities are stored in the same cache, we potentially have a cache
>>     that contains data belonging to multiple cache loaders. And the
>>     reason for all this is cos we don't support asymmetric clusters.
>>
>>     Could someone start a design wiki to grab all the requirements?
>>
>>      >
>>      > getCache( cacheName, classLoader ) would return a delegate to the
>>      > original cache, having a specific marshaller in the invocation
>>     context
>>      > as Trustin was suggesting.
>>      >
>>      > Cheers,
>>      > Sanne
>>      >
>>      >
>>      > 2011/5/16 Pete Muir mailto:pm...@redhat.com>>:
>>      >>
>>      >> 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
>>     mailto:da...@redhat.com>> wrote:
>>      > On 11/05/2011 17:54, Dan Berindei wrote:
>>      >> On Wed, May 11, 2011 at 7:08 PM, Pete Muir>     >  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 

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

[David M. Lloyd]

I know, we can just attach the class loader to the cache!

Okay, just kidding, but Galder is right, this conversation is going in 
circles.  We already discussed that in this thread and a number of 
points were raised for and against.

On 05/16/2011 01:20 PM, Adrian Cole wrote:
> What about a helper that just returns a cache with a specific
> classloader from a cache?
>
> cache.withLoader(cl).get(K key)
>
> -a
>
>
> On Mon, May 16, 2011 at 11:14 AM, Galder Zamarreño  > wrote:
>
>
> On May 16, 2011, at 7:57 PM, Sanne Grinovero wrote:
>
>  > I don't like the TCCL either, so I'll repeat my suggestion from two
>  > weeks ago to just have:
>  >
>  > Cache c = cacheManager.getCache( cacheName, classLoader );
>  >
>  > sounds reasonable to me to have the application declare it's
> intentions once ?
>  >
>  > BTW I don't like
>  >
>  > "cache.get(K key, Class clazz)"
>  >
>  > as we're not speaking only about the get(K) method, but about many
>  > methods and this will explode the number of method of Cache; on the
>  > other hand I think it;s acceptable to have a single Cache instance
>  > used by a single application/classloader. You can still have multiple
>  > applications share the same underlying cache and use different
>  > classloaders:
>
> Guys, we're going around in circles. As I said the other week, you
> can't assume 1 cache = 1 classloader cos for example in the
> Hibernate 2LC all entities will be stored in a single cache as
> opposed to today where we have a cache per entity. And if all
> entities are stored in the same cache, we potentially have a cache
> that contains data belonging to multiple cache loaders. And the
> reason for all this is cos we don't support asymmetric clusters.
>
> Could someone start a design wiki to grab all the requirements?
>
>  >
>  > getCache( cacheName, classLoader ) would return a delegate to the
>  > original cache, having a specific marshaller in the invocation
> context
>  > as Trustin was suggesting.
>  >
>  > Cheers,
>  > Sanne
>  >
>  >
>  > 2011/5/16 Pete Muir mailto:pm...@redhat.com>>:
>  >>
>  >> 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
> mailto:da...@redhat.com>> wrote:
>  > On 11/05/2011 17:54, Dan Berindei wrote:
>  >> On Wed, May 11, 2011 at 7:08 PM, Pete Muir >  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.cl

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

[Pete Muir]

BTW having thought about it more, this seems to be the only option that 
actually works at the API level, even if it will require some re-eng in 
Infinispan core.

On 16 May 2011, at 19:29, Pete Muir wrote:

> 
> On 16 May 2011, at 19:24, Sanne Grinovero wrote:
> 
>> 2011/5/16 Pete Muir :
>>> This is the "hibernate session style contract" that Jason is talking about. 
>>>  As the CM can be shared (e.g. in JNDI), then the same Cache object can be 
>>> returned in multiple applications in the app server, meaning you can't 
>>> simply associate the CL with a Cache object when it's created.
>> 
>> A CM could be JNDI registered, but a Cache is not registered. assuming
>> same cacheManager, two different applications could do:
>> 
>> cacheA = cm.getCache( "hibernateCache", appAClassLoader );
>> cacheB = cm.getCache( "hibernateCache", appBClassLoader );
>> 
>> cacheA != cacheB
>> still they will delegate toe the same "hibernateCache", but being
>> different only in which ClassLoader they set in their invocation
>> context, which will be read by the Unmarshaller.
> 
> AIUI you just said what I said in different language?
> 
> 
> 
> ___
> infinispan-dev mailing list
> infinispan-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev


___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Pete Muir]

On 16 May 2011, at 19:24, Sanne Grinovero wrote:

> 2011/5/16 Pete Muir :
>> This is the "hibernate session style contract" that Jason is talking about.  
>> As the CM can be shared (e.g. in JNDI), then the same Cache object can be 
>> returned in multiple applications in the app server, meaning you can't 
>> simply associate the CL with a Cache object when it's created.
> 
> A CM could be JNDI registered, but a Cache is not registered. assuming
> same cacheManager, two different applications could do:
> 
> cacheA = cm.getCache( "hibernateCache", appAClassLoader );
> cacheB = cm.getCache( "hibernateCache", appBClassLoader );
> 
> cacheA != cacheB
> still they will delegate toe the same "hibernateCache", but being
> different only in which ClassLoader they set in their invocation
> context, which will be read by the Unmarshaller.

AIUI you just said what I said in different language?



___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Sanne Grinovero]

2011/5/16 Pete Muir :
> This is the "hibernate session style contract" that Jason is talking about.  
> As the CM can be shared (e.g. in JNDI), then the same Cache object can be 
> returned in multiple applications in the app server, meaning you can't simply 
> associate the CL with a Cache object when it's created.

A CM could be JNDI registered, but a Cache is not registered. assuming
same cacheManager, two different applications could do:

cacheA = cm.getCache( "hibernateCache", appAClassLoader );
cacheB = cm.getCache( "hibernateCache", appBClassLoader );

cacheA != cacheB
still they will delegate toe the same "hibernateCache", but being
different only in which ClassLoader they set in their invocation
context, which will be read by the Unmarshaller.


Cheers,
Sanne


>
> On 16 May 2011, at 18:57, Sanne Grinovero wrote:
>
>> I don't like the TCCL either, so I'll repeat my suggestion from two
>> weeks ago to just have:
>>
>> Cache c = cacheManager.getCache( cacheName, classLoader );
>>
>> sounds reasonable to me to have the application declare it's intentions once 
>> ?
>>
>> BTW I don't like
>>
>> "cache.get(K key, Class clazz)"
>>
>> as we're not speaking only about the get(K) method, but about many
>> methods and this will explode the number of method of Cache; on the
>> other hand I think it;s acceptable to have a single Cache instance
>> used by a single application/classloader. You can still have multiple
>> applications share the same underlying cache and use different
>> classloaders:
>>
>> getCache( cacheName, classLoader ) would return a delegate to the
>> original cache, having a specific marshaller in the invocation context
>> as Trustin was suggesting.
>>
>> Cheers,
>> Sanne
>>
>>
>> 2011/5/16 Pete Muir :
>>>
>>> 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  
> wrote:
>> On 11/05/2011 17:54, Dan Berindei wrote:
>>> On Wed, May 11, 2011 at 7:08 PM, Pete Muir  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 

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

[Adrian Cole]

What about a helper that just returns a cache with a specific classloader
from a cache?

cache.withLoader(cl).get(K key)

-a


On Mon, May 16, 2011 at 11:14 AM, Galder Zamarreño wrote:

>
> On May 16, 2011, at 7:57 PM, Sanne Grinovero wrote:
>
> > I don't like the TCCL either, so I'll repeat my suggestion from two
> > weeks ago to just have:
> >
> > Cache c = cacheManager.getCache( cacheName, classLoader );
> >
> > sounds reasonable to me to have the application declare it's intentions
> once ?
> >
> > BTW I don't like
> >
> > "cache.get(K key, Class clazz)"
> >
> > as we're not speaking only about the get(K) method, but about many
> > methods and this will explode the number of method of Cache; on the
> > other hand I think it;s acceptable to have a single Cache instance
> > used by a single application/classloader. You can still have multiple
> > applications share the same underlying cache and use different
> > classloaders:
>
> Guys, we're going around in circles. As I said the other week, you can't
> assume 1 cache = 1 classloader cos for example in the Hibernate 2LC all
> entities will be stored in a single cache as opposed to today where we have
> a cache per entity. And if all entities are stored in the same cache, we
> potentially have a cache that contains data belonging to multiple cache
> loaders. And the reason for all this is cos we don't support asymmetric
> clusters.
>
> Could someone start a design wiki to grab all the requirements?
>
> >
> > getCache( cacheName, classLoader ) would return a delegate to the
> > original cache, having a specific marshaller in the invocation context
> > as Trustin was suggesting.
> >
> > Cheers,
> > Sanne
> >
> >
> > 2011/5/16 Pete Muir :
> >>
> >> 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 
> wrote:
> > On 11/05/2011 17:54, Dan Berindei wrote:
> >> On Wed, May 11, 2011 at 7:08 PM, Pete Muir
>  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 

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

[Emmanuel Bernard]

I don't enjoy using JBoss Logging for that reason :)
I always have to get down to the JavaDoc or figure out the list of available 
params to understand what debug* does for example and which one I need.

I guess that's taste and for something like JBoss Logging, a fluent API is 
costly (unnecessary object creation etc).

On 16 mai 2011, at 20:01, David M. Lloyd wrote:

> As a rule I've never cared about exploding the number of methods on an 
> API class as long as:
> 
> 1. There are few basic functions - i.e. get is get, and regardless of 
> which overload we're talking about it behaves consistently - and the 
> large number of methods is due mainly to overloads
> 2. The end user is not expected to ever need to implement the interface
> 
> For example, org.jboss.logging.Logger has about a zillion methods but 
> they all boil down to: log, trace, debug, info, warn, error, fatal.  So, 
> there's relatively little confusion possible.
> 
> Just FWIW...
> 
> On 05/16/2011 12:57 PM, Sanne Grinovero wrote:
>> I don't like the TCCL either, so I'll repeat my suggestion from two
>> weeks ago to just have:
>> 
>> Cache c = cacheManager.getCache( cacheName, classLoader );
>> 
>> sounds reasonable to me to have the application declare it's intentions once 
>> ?
>> 
>> BTW I don't like
>> 
>> "cache.get(K key, Class  clazz)"
>> 
>> as we're not speaking only about the get(K) method, but about many
>> methods and this will explode the number of method of Cache; on the
>> other hand I think it;s acceptable to have a single Cache instance
>> used by a single application/classloader. You can still have multiple
>> applications share the same underlying cache and use different
>> classloaders:
>> 
>> getCache( cacheName, classLoader ) would return a delegate to the
>> original cache, having a specific marshaller in the invocation context
>> as Trustin was suggesting.
>> 
>> Cheers,
>> Sanne
>> 
>> 
>> 2011/5/16 Pete Muir:
>>> 
>>> 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  
> wrote:
>> On 11/05/2011 17:54, Dan Berindei wrote:
>>> On Wed, May 11, 2011 at 7:08 PM, Pete Muirwrote:
 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 Co

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

[Galder Zamarreño]

On May 16, 2011, at 7:57 PM, Sanne Grinovero wrote:

> I don't like the TCCL either, so I'll repeat my suggestion from two
> weeks ago to just have:
> 
> Cache c = cacheManager.getCache( cacheName, classLoader );
> 
> sounds reasonable to me to have the application declare it's intentions once ?
> 
> BTW I don't like
> 
> "cache.get(K key, Class clazz)"
> 
> as we're not speaking only about the get(K) method, but about many
> methods and this will explode the number of method of Cache; on the
> other hand I think it;s acceptable to have a single Cache instance
> used by a single application/classloader. You can still have multiple
> applications share the same underlying cache and use different
> classloaders:

Guys, we're going around in circles. As I said the other week, you can't assume 
1 cache = 1 classloader cos for example in the Hibernate 2LC all entities will 
be stored in a single cache as opposed to today where we have a cache per 
entity. And if all entities are stored in the same cache, we potentially have a 
cache that contains data belonging to multiple cache loaders. And the reason 
for all this is cos we don't support asymmetric clusters.

Could someone start a design wiki to grab all the requirements?

> 
> getCache( cacheName, classLoader ) would return a delegate to the
> original cache, having a specific marshaller in the invocation context
> as Trustin was suggesting.
> 
> Cheers,
> Sanne
> 
> 
> 2011/5/16 Pete Muir :
>> 
>> 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  
 wrote:
> On 11/05/2011 17:54, Dan Berindei wrote:
>> On Wed, May 11, 2011 at 7:08 PM, Pete Muir  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 detec

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

[Pete Muir]

This is the "hibernate session style contract" that Jason is talking about.  As 
the CM can be shared (e.g. in JNDI), then the same Cache object can be returned 
in multiple applications in the app server, meaning you can't simply associate 
the CL with a Cache object when it's created.

On 16 May 2011, at 18:57, Sanne Grinovero wrote:

> I don't like the TCCL either, so I'll repeat my suggestion from two
> weeks ago to just have:
> 
> Cache c = cacheManager.getCache( cacheName, classLoader );
> 
> sounds reasonable to me to have the application declare it's intentions once ?
> 
> BTW I don't like
> 
> "cache.get(K key, Class clazz)"
> 
> as we're not speaking only about the get(K) method, but about many
> methods and this will explode the number of method of Cache; on the
> other hand I think it;s acceptable to have a single Cache instance
> used by a single application/classloader. You can still have multiple
> applications share the same underlying cache and use different
> classloaders:
> 
> getCache( cacheName, classLoader ) would return a delegate to the
> original cache, having a specific marshaller in the invocation context
> as Trustin was suggesting.
> 
> Cheers,
> Sanne
> 
> 
> 2011/5/16 Pete Muir :
>> 
>> 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  
 wrote:
> On 11/05/2011 17:54, Dan Berindei wrote:
>> On Wed, May 11, 2011 at 7:08 PM, Pete Muir  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 muc

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

[David M. Lloyd]

As a rule I've never cared about exploding the number of methods on an 
API class as long as:

1. There are few basic functions - i.e. get is get, and regardless of 
which overload we're talking about it behaves consistently - and the 
large number of methods is due mainly to overloads
2. The end user is not expected to ever need to implement the interface

For example, org.jboss.logging.Logger has about a zillion methods but 
they all boil down to: log, trace, debug, info, warn, error, fatal.  So, 
there's relatively little confusion possible.

Just FWIW...

On 05/16/2011 12:57 PM, Sanne Grinovero wrote:
> I don't like the TCCL either, so I'll repeat my suggestion from two
> weeks ago to just have:
>
> Cache c = cacheManager.getCache( cacheName, classLoader );
>
> sounds reasonable to me to have the application declare it's intentions once ?
>
> BTW I don't like
>
> "cache.get(K key, Class  clazz)"
>
> as we're not speaking only about the get(K) method, but about many
> methods and this will explode the number of method of Cache; on the
> other hand I think it;s acceptable to have a single Cache instance
> used by a single application/classloader. You can still have multiple
> applications share the same underlying cache and use different
> classloaders:
>
> getCache( cacheName, classLoader ) would return a delegate to the
> original cache, having a specific marshaller in the invocation context
> as Trustin was suggesting.
>
> Cheers,
> Sanne
>
>
> 2011/5/16 Pete Muir:
>>
>> 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  
 wrote:
> On 11/05/2011 17:54, Dan Berindei wrote:
>> On Wed, May 11, 2011 at 7:08 PM, Pete Muirwrote:
>>> 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:
>>>
>>>

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

[Sanne Grinovero]

I don't like the TCCL either, so I'll repeat my suggestion from two
weeks ago to just have:

Cache c = cacheManager.getCache( cacheName, classLoader );

sounds reasonable to me to have the application declare it's intentions once ?

BTW I don't like

"cache.get(K key, Class clazz)"

as we're not speaking only about the get(K) method, but about many
methods and this will explode the number of method of Cache; on the
other hand I think it;s acceptable to have a single Cache instance
used by a single application/classloader. You can still have multiple
applications share the same underlying cache and use different
classloaders:

getCache( cacheName, classLoader ) would return a delegate to the
original cache, having a specific marshaller in the invocation context
as Trustin was suggesting.

Cheers,
Sanne


2011/5/16 Pete Muir :
>
> 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  wrote:
 On 11/05/2011 17:54, Dan Berindei wrote:
> On Wed, May 11, 2011 at 7:08 PM, Pete Muir  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@lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
> ___
> infinispan-dev mailing list
> infinispan-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>

__

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

[Pete Muir]

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  wrote:
>>> On 11/05/2011 17:54, Dan Berindei wrote:
 On Wed, May 11, 2011 at 7:08 PM, Pete Muir  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@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev


___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Galder Zamarreño]

On May 12, 2011, at 11:18 AM, Dan Berindei wrote:

> On Wed, May 11, 2011 at 11:18 PM, David Bosschaert  wrote:
>> On 11/05/2011 17:54, Dan Berindei wrote:
>>> On Wed, May 11, 2011 at 7:08 PM, Pete Muir  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. 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@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Dan Berindei]

On Fri, May 13, 2011 at 5:48 PM, Jason T. Greene
 wrote:
> On 5/12/11 4:18 AM, Dan Berindei wrote:
>>
>> On Wed, May 11, 2011 at 11:18 PM, David Bosschaert
>>  wrote:
>>>
>>> On 11/05/2011 17:54, Dan Berindei wrote:

 On Wed, May 11, 2011 at 7:08 PM, Pete Muir    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

___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Jason T. Greene]

On 5/12/11 4:18 AM, Dan Berindei wrote:
> On Wed, May 11, 2011 at 11:18 PM, David Bosschaert  wrote:
>> On 11/05/2011 17:54, Dan Berindei wrote:
>>> On Wed, May 11, 2011 at 7:08 PM, Pete Muirwrote:
 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.

> 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.

-- 
Jason T. Greene
JBoss, a division of Red Hat
___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Dan Berindei]

On Thu, May 12, 2011 at 1:22 PM, Emmanuel Bernard
 wrote:
>
> On 12 mai 2011, at 11:18, Dan Berindei wrote:
>
>> 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.
>
> Note that I don't think it works in the case you mentioned earlier ie a 
> ArrayList with classes from many different bundles.
>

It works, only now it's the user's job to provide the right classloader.

I suspect most people will have the same situation as Augusto, the guy
who was asking about classloading rules on the forum
(http://community.jboss.org/message/602759): they already have a
bundle that has access to the classes that will ever be in the cache
(in Augusto's case via DynamicImport-Package: *), their only problem
is convincing Infinispan to use that bundle's classloader for
unmarshalling.

If they don't like the idea of having a catch-all bundle, they can
instead write a bridge classloader themselves that holds a list of
classloaders and loads classes from any of them. They can even write
the bundle ids in the cache if the list of bundles can only be
determined at runtime.

Of course, Augusto's bundles don't access each other's objects in the
cache, so his problem could also be solved by giving each bundle it's
own CacheDelegateWithContext (or CacheDelegateSession, if you will)
instance configured with that bundle's classloader, removing the need
for the DynamicImport-Package directive in his central bundle.

Cheers
Dan
___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Emmanuel Bernard]

On 12 mai 2011, at 11:18, Dan Berindei wrote:

> 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.

Note that I don't think it works in the case you mentioned earlier ie a 
ArrayList with classes from many different bundles.
___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Dan Berindei]

On Wed, May 11, 2011 at 11:18 PM, David Bosschaert  wrote:
> On 11/05/2011 17:54, Dan Berindei wrote:
>> On Wed, May 11, 2011 at 7:08 PM, Pete Muir  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.


Cheers
Dan

___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[David Bosschaert]

On 11/05/2011 17:54, Dan Berindei wrote:
> On Wed, May 11, 2011 at 7:08 PM, Pete Muir  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?

Cheers,

David
___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Dan Berindei]

On Wed, May 11, 2011 at 7:08 PM, Pete Muir  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.


>
> On 9 May 2011, at 20:57, David Bosschaert wrote:
>
>> Yeah, the bottom line is that the value of the TCCL is not defined by
>> the OSGi standards. You can define it yourself by setting it before you
>> call the relevant code but as said before this is ugly and it's actually
>> very error-prone because there's nothing in the API to remind you to do
>> it, so you might forget the odd time (it's effectively a hidden argument
>> to the API).
>> To make things even worse, there is a chance that the TCCL is not unset
>> after previous use which means that its value might happen to be correct
>> in some cases and incorrect in others - which is what the issue in the
>> post seems to be hitting.
>>
>> I think making this part of the API explicit is the safest option. Yes,
>> it requires people to think about classloading who don't like to think
>> about classloading, but generally passing in getClass().getClassLoader()
>> is all that's needed to get the right one...
>>
>> Just my 2c,
>>
>> David
>>
>> On 09/05/2011 18:01, Galder Zamarreño wrote:
>>> Guys,
>>>
>>> It's funny that we're talking about this but this appears to be precisely 
>>> the use case that we don't support as per user forum post in: 
>>> http://community.jboss.org/thread/166080?tstart=0
>>>
>>> Pete, do we have a JIRA for this? If not, would you mind creating one?
>>>
>>> Cheers,
>>>
>>> On May 9, 2011, at 5:37 PM, Pete Muir wrote:
>>>
 Ok, let me stew on that ;-)

 On 9 May 2011, at 16:35, David Bosschaert wrote:

> If you have a Bundle object or BundleContext object you can figure out 
> what classloader is associated with that. Also, if you have a class from 
> a bundle you can find out what it's classloader is (obviously).
>
> However, you need to have one of those things to find this out. There is 
> nothing magically available in the current context to tell you what the 
> current Bundle is.
> It's generally really easy for the caller though to find out what the 
> classloader is, though... If you have bundle X, you'd know a class from 
> that bundle a.b.c.D (any class will do). Then you can simply call 
> D.class.getClassLoader() and you've got the Bundle Classloader.
>
> Hope this helps,
>
> David
>
> On 09/05/2011 16:27, Pete Muir wrote:
>> The issue that David raises is that in an OSGi env that this wouldn't be 
>> done by OSGi so would be up to the user or would require some extra 
>> integration library. I'm not even sure if this is possible in OSGi? 
>> David, is there anyway for a framework to "find out" the current bundle 
>> classloader in OSGi rather than having to be told it (i.e. push rather 
>> than pull)?
>>
>> The idea is that in AS that by doing (a) it would require the 
>> integration to make sure the TCCL is set before Infinispan is called 
>> (this is the way many things are integrated into GF for example).
>>
>> On 5 May 2011, at 20:05, Emmanuel Bernard wrote:
>>
>>> Quick question.
>>> In case of 2.a (ie setting the TCCL), this is a requirement for 
>>> frameworks and libraries using Infinispan, right? Not / never for a 
>>> user application using Infinispan (in this case it seems that the 
>>> application class loader will do the right thing and is set as the TCCL 
>>> by the AS environment)?
>>>
>>> Emmanuel
>>>
>>> On 5 mai 2011, at 10:55, Pete Muir wrote:
>>>
 I talked about this with Emmanuel last night, and we were

 a) concerned about the pollution of the API that this implies
 b) not sure why we need to do this

 So I also spoke to Jason to understand his initial motivation, and 
 from this chat I think it's clear that things have gone off course 
 here.

 Jason raised two issues with modularity/classloading in Infinispan:

 1) Using the TCCL as the classloader to load Infinispan classes is 

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

[Pete Muir]

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.

On 9 May 2011, at 20:57, David Bosschaert wrote:

> Yeah, the bottom line is that the value of the TCCL is not defined by 
> the OSGi standards. You can define it yourself by setting it before you 
> call the relevant code but as said before this is ugly and it's actually 
> very error-prone because there's nothing in the API to remind you to do 
> it, so you might forget the odd time (it's effectively a hidden argument 
> to the API).
> To make things even worse, there is a chance that the TCCL is not unset 
> after previous use which means that its value might happen to be correct 
> in some cases and incorrect in others - which is what the issue in the 
> post seems to be hitting.
> 
> I think making this part of the API explicit is the safest option. Yes, 
> it requires people to think about classloading who don't like to think 
> about classloading, but generally passing in getClass().getClassLoader() 
> is all that's needed to get the right one...
> 
> Just my 2c,
> 
> David
> 
> On 09/05/2011 18:01, Galder Zamarreño wrote:
>> Guys,
>> 
>> It's funny that we're talking about this but this appears to be precisely 
>> the use case that we don't support as per user forum post in: 
>> http://community.jboss.org/thread/166080?tstart=0
>> 
>> Pete, do we have a JIRA for this? If not, would you mind creating one?
>> 
>> Cheers,
>> 
>> On May 9, 2011, at 5:37 PM, Pete Muir wrote:
>> 
>>> Ok, let me stew on that ;-)
>>> 
>>> On 9 May 2011, at 16:35, David Bosschaert wrote:
>>> 
 If you have a Bundle object or BundleContext object you can figure out 
 what classloader is associated with that. Also, if you have a class from a 
 bundle you can find out what it's classloader is (obviously).
 
 However, you need to have one of those things to find this out. There is 
 nothing magically available in the current context to tell you what the 
 current Bundle is.
 It's generally really easy for the caller though to find out what the 
 classloader is, though... If you have bundle X, you'd know a class from 
 that bundle a.b.c.D (any class will do). Then you can simply call 
 D.class.getClassLoader() and you've got the Bundle Classloader.
 
 Hope this helps,
 
 David
 
 On 09/05/2011 16:27, Pete Muir wrote:
> The issue that David raises is that in an OSGi env that this wouldn't be 
> done by OSGi so would be up to the user or would require some extra 
> integration library. I'm not even sure if this is possible in OSGi? 
> David, is there anyway for a framework to "find out" the current bundle 
> classloader in OSGi rather than having to be told it (i.e. push rather 
> than pull)?
> 
> The idea is that in AS that by doing (a) it would require the integration 
> to make sure the TCCL is set before Infinispan is called (this is the way 
> many things are integrated into GF for example).
> 
> On 5 May 2011, at 20:05, Emmanuel Bernard wrote:
> 
>> Quick question.
>> In case of 2.a (ie setting the TCCL), this is a requirement for 
>> frameworks and libraries using Infinispan, right? Not / never for a user 
>> application using Infinispan (in this case it seems that the application 
>> class loader will do the right thing and is set as the TCCL by the AS 
>> environment)?
>> 
>> Emmanuel
>> 
>> On 5 mai 2011, at 10:55, Pete Muir wrote:
>> 
>>> I talked about this with Emmanuel last night, and we were
>>> 
>>> a) concerned about the pollution of the API that this implies
>>> b) not sure why we need to do this
>>> 
>>> So I also spoke to Jason to understand his initial motivation, and from 
>>> this chat I think it's clear that things have gone off course here.
>>> 
>>> Jason raised two issues with modularity/classloading in Infinispan:
>>> 
>>> 1) Using the TCCL as the classloader to load Infinispan classes is a 
>>> bad idea. Instead we should use 
>>> org.infinispan.foo.Bar.getClass().getClassLoader().
>>> 
>>> 2) When we need to load application classes we need a different 
>>> approach to that used today. Most of the time the TCCL is perfect for 
>>> this. However *sometimes* Infinispan may 

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

[Pete Muir]

On 9 May 2011, at 18:02, Galder Zamarreño wrote:

> 
> On May 9, 2011, at 7:01 PM, Galder Zamarreño wrote:
> 
>> Guys,
>> 
>> It's funny that we're talking about this but this appears to be precisely 
>> the use case that we don't support as per user forum post in: 
>> http://community.jboss.org/thread/166080?tstart=0
>> 
>> Pete, do we have a JIRA for this? If not, would you mind creating one?
> 
> If not, would you mind creating one?

It's https://issues.jboss.org/browse/ISPN-1096

> 
>> 
>> Cheers,
>> 
>> On May 9, 2011, at 5:37 PM, Pete Muir wrote:
>> 
>>> Ok, let me stew on that ;-)
>>> 
>>> On 9 May 2011, at 16:35, David Bosschaert wrote:
>>> 
 If you have a Bundle object or BundleContext object you can figure out 
 what classloader is associated with that. Also, if you have a class from a 
 bundle you can find out what it's classloader is (obviously).
 
 However, you need to have one of those things to find this out. There is 
 nothing magically available in the current context to tell you what the 
 current Bundle is.
 It's generally really easy for the caller though to find out what the 
 classloader is, though... If you have bundle X, you'd know a class from 
 that bundle a.b.c.D (any class will do). Then you can simply call 
 D.class.getClassLoader() and you've got the Bundle Classloader.
 
 Hope this helps,
 
 David
 
 On 09/05/2011 16:27, Pete Muir wrote:
> The issue that David raises is that in an OSGi env that this wouldn't be 
> done by OSGi so would be up to the user or would require some extra 
> integration library. I'm not even sure if this is possible in OSGi? 
> David, is there anyway for a framework to "find out" the current bundle 
> classloader in OSGi rather than having to be told it (i.e. push rather 
> than pull)?
> 
> The idea is that in AS that by doing (a) it would require the integration 
> to make sure the TCCL is set before Infinispan is called (this is the way 
> many things are integrated into GF for example).
> 
> On 5 May 2011, at 20:05, Emmanuel Bernard wrote:
> 
>> Quick question.
>> In case of 2.a (ie setting the TCCL), this is a requirement for 
>> frameworks and libraries using Infinispan, right? Not / never for a user 
>> application using Infinispan (in this case it seems that the application 
>> class loader will do the right thing and is set as the TCCL by the AS 
>> environment)?
>> 
>> Emmanuel
>> 
>> On 5 mai 2011, at 10:55, Pete Muir wrote:
>> 
>>> I talked about this with Emmanuel last night, and we were
>>> 
>>> a) concerned about the pollution of the API that this implies
>>> b) not sure why we need to do this
>>> 
>>> So I also spoke to Jason to understand his initial motivation, and from 
>>> this chat I think it's clear that things have gone off course here.
>>> 
>>> Jason raised two issues with modularity/classloading in Infinispan:
>>> 
>>> 1) Using the TCCL as the classloader to load Infinispan classes is a 
>>> bad idea. Instead we should use 
>>> org.infinispan.foo.Bar.getClass().getClassLoader().
>>> 
>>> 2) When we need to load application classes we need a different 
>>> approach to that used today. Most of the time the TCCL is perfect for 
>>> this. However *sometimes* Infinispan may be invoked outside of the 
>>> application, when the TCCL may not be set in AS7. Jason and I discussed 
>>> three options:
>>> 
>>> a) require (through a platform integration documentation contract) that 
>>> the TCCL must always be set when Infinispan is invoked.
>>> b) Have some sort of InvocationContext which knows what the correct 
>>> classloader to use is (aka Hibernate/Seam/Weld design where there is a 
>>> per-application construct based on a ThreadLocal). Given this hasn't 
>>> been designed into the core, it seems like a large retrofit
>>> c) Make users specify the CL (directly or indirectly) via the API (as 
>>> we discussed).
>>> 
>>> Personally I think that (a) is the best approach right now, and is not 
>>> that onerous a requirement.
>>> 
>>> We might want to make the TCCL usage pluggable for OSGI envs. Cc'd 
>>> David to get his feedback.
>>> 
>>> On 4 May 2011, at 10:46, Dan Berindei wrote:
>>> 
 On Wed, May 4, 2011 at 5:09 PM, Pete Muir  wrote:
> On 4 May 2011, at 05:34, Dan Berindei wrote:
> 
>> On Wed, May 4, 2011 at 10:14 AM, Galder Zamarreño 
>>  wrote:
>>> On May 3, 2011, at 2:33 PM, Sanne Grinovero wrote:
>>> 
 2011/5/3 "이희승 (Trustin Lee)":
> On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
>> 2011/5/2 Manik Surtani:
>>> On 1 May 2011, at 13:38, Pete Muir wrote:
>>> 
>> As in, user API?  That's a little 

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

[David Bosschaert]

Yeah, the bottom line is that the value of the TCCL is not defined by 
the OSGi standards. You can define it yourself by setting it before you 
call the relevant code but as said before this is ugly and it's actually 
very error-prone because there's nothing in the API to remind you to do 
it, so you might forget the odd time (it's effectively a hidden argument 
to the API).
To make things even worse, there is a chance that the TCCL is not unset 
after previous use which means that its value might happen to be correct 
in some cases and incorrect in others - which is what the issue in the 
post seems to be hitting.

I think making this part of the API explicit is the safest option. Yes, 
it requires people to think about classloading who don't like to think 
about classloading, but generally passing in getClass().getClassLoader() 
is all that's needed to get the right one...

Just my 2c,

David

On 09/05/2011 18:01, Galder Zamarreño wrote:
> Guys,
>
> It's funny that we're talking about this but this appears to be precisely the 
> use case that we don't support as per user forum post in: 
> http://community.jboss.org/thread/166080?tstart=0
>
> Pete, do we have a JIRA for this? If not, would you mind creating one?
>
> Cheers,
>
> On May 9, 2011, at 5:37 PM, Pete Muir wrote:
>
>> Ok, let me stew on that ;-)
>>
>> On 9 May 2011, at 16:35, David Bosschaert wrote:
>>
>>> If you have a Bundle object or BundleContext object you can figure out what 
>>> classloader is associated with that. Also, if you have a class from a 
>>> bundle you can find out what it's classloader is (obviously).
>>>
>>> However, you need to have one of those things to find this out. There is 
>>> nothing magically available in the current context to tell you what the 
>>> current Bundle is.
>>> It's generally really easy for the caller though to find out what the 
>>> classloader is, though... If you have bundle X, you'd know a class from 
>>> that bundle a.b.c.D (any class will do). Then you can simply call 
>>> D.class.getClassLoader() and you've got the Bundle Classloader.
>>>
>>> Hope this helps,
>>>
>>> David
>>>
>>> On 09/05/2011 16:27, Pete Muir wrote:
 The issue that David raises is that in an OSGi env that this wouldn't be 
 done by OSGi so would be up to the user or would require some extra 
 integration library. I'm not even sure if this is possible in OSGi? David, 
 is there anyway for a framework to "find out" the current bundle 
 classloader in OSGi rather than having to be told it (i.e. push rather 
 than pull)?

 The idea is that in AS that by doing (a) it would require the integration 
 to make sure the TCCL is set before Infinispan is called (this is the way 
 many things are integrated into GF for example).

 On 5 May 2011, at 20:05, Emmanuel Bernard wrote:

> Quick question.
> In case of 2.a (ie setting the TCCL), this is a requirement for 
> frameworks and libraries using Infinispan, right? Not / never for a user 
> application using Infinispan (in this case it seems that the application 
> class loader will do the right thing and is set as the TCCL by the AS 
> environment)?
>
> Emmanuel
>
> On 5 mai 2011, at 10:55, Pete Muir wrote:
>
>> I talked about this with Emmanuel last night, and we were
>>
>> a) concerned about the pollution of the API that this implies
>> b) not sure why we need to do this
>>
>> So I also spoke to Jason to understand his initial motivation, and from 
>> this chat I think it's clear that things have gone off course here.
>>
>> Jason raised two issues with modularity/classloading in Infinispan:
>>
>> 1) Using the TCCL as the classloader to load Infinispan classes is a bad 
>> idea. Instead we should use 
>> org.infinispan.foo.Bar.getClass().getClassLoader().
>>
>> 2) When we need to load application classes we need a different approach 
>> to that used today. Most of the time the TCCL is perfect for this. 
>> However *sometimes* Infinispan may be invoked outside of the 
>> application, when the TCCL may not be set in AS7. Jason and I discussed 
>> three options:
>>
>> a) require (through a platform integration documentation contract) that 
>> the TCCL must always be set when Infinispan is invoked.
>> b) Have some sort of InvocationContext which knows what the correct 
>> classloader to use is (aka Hibernate/Seam/Weld design where there is a 
>> per-application construct based on a ThreadLocal). Given this hasn't 
>> been designed into the core, it seems like a large retrofit
>> c) Make users specify the CL (directly or indirectly) via the API (as we 
>> discussed).
>>
>> Personally I think that (a) is the best approach right now, and is not 
>> that onerous a requirement.
>>
>> We might want to make the TCCL usage pluggable for OSGI envs. Cc'd David 
>> to get his f

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

[Galder Zamarreño]

On May 9, 2011, at 7:01 PM, Galder Zamarreño wrote:

> Guys,
> 
> It's funny that we're talking about this but this appears to be precisely the 
> use case that we don't support as per user forum post in: 
> http://community.jboss.org/thread/166080?tstart=0
> 
> Pete, do we have a JIRA for this? If not, would you mind creating one?

If not, would you mind creating one?

> 
> Cheers,
> 
> On May 9, 2011, at 5:37 PM, Pete Muir wrote:
> 
>> Ok, let me stew on that ;-)
>> 
>> On 9 May 2011, at 16:35, David Bosschaert wrote:
>> 
>>> If you have a Bundle object or BundleContext object you can figure out what 
>>> classloader is associated with that. Also, if you have a class from a 
>>> bundle you can find out what it's classloader is (obviously).
>>> 
>>> However, you need to have one of those things to find this out. There is 
>>> nothing magically available in the current context to tell you what the 
>>> current Bundle is.
>>> It's generally really easy for the caller though to find out what the 
>>> classloader is, though... If you have bundle X, you'd know a class from 
>>> that bundle a.b.c.D (any class will do). Then you can simply call 
>>> D.class.getClassLoader() and you've got the Bundle Classloader.
>>> 
>>> Hope this helps,
>>> 
>>> David
>>> 
>>> On 09/05/2011 16:27, Pete Muir wrote:
 The issue that David raises is that in an OSGi env that this wouldn't be 
 done by OSGi so would be up to the user or would require some extra 
 integration library. I'm not even sure if this is possible in OSGi? David, 
 is there anyway for a framework to "find out" the current bundle 
 classloader in OSGi rather than having to be told it (i.e. push rather 
 than pull)?
 
 The idea is that in AS that by doing (a) it would require the integration 
 to make sure the TCCL is set before Infinispan is called (this is the way 
 many things are integrated into GF for example).
 
 On 5 May 2011, at 20:05, Emmanuel Bernard wrote:
 
> Quick question.
> In case of 2.a (ie setting the TCCL), this is a requirement for 
> frameworks and libraries using Infinispan, right? Not / never for a user 
> application using Infinispan (in this case it seems that the application 
> class loader will do the right thing and is set as the TCCL by the AS 
> environment)?
> 
> Emmanuel
> 
> On 5 mai 2011, at 10:55, Pete Muir wrote:
> 
>> I talked about this with Emmanuel last night, and we were
>> 
>> a) concerned about the pollution of the API that this implies
>> b) not sure why we need to do this
>> 
>> So I also spoke to Jason to understand his initial motivation, and from 
>> this chat I think it's clear that things have gone off course here.
>> 
>> Jason raised two issues with modularity/classloading in Infinispan:
>> 
>> 1) Using the TCCL as the classloader to load Infinispan classes is a bad 
>> idea. Instead we should use 
>> org.infinispan.foo.Bar.getClass().getClassLoader().
>> 
>> 2) When we need to load application classes we need a different approach 
>> to that used today. Most of the time the TCCL is perfect for this. 
>> However *sometimes* Infinispan may be invoked outside of the 
>> application, when the TCCL may not be set in AS7. Jason and I discussed 
>> three options:
>> 
>> a) require (through a platform integration documentation contract) that 
>> the TCCL must always be set when Infinispan is invoked.
>> b) Have some sort of InvocationContext which knows what the correct 
>> classloader to use is (aka Hibernate/Seam/Weld design where there is a 
>> per-application construct based on a ThreadLocal). Given this hasn't 
>> been designed into the core, it seems like a large retrofit
>> c) Make users specify the CL (directly or indirectly) via the API (as we 
>> discussed).
>> 
>> Personally I think that (a) is the best approach right now, and is not 
>> that onerous a requirement.
>> 
>> We might want to make the TCCL usage pluggable for OSGI envs. Cc'd David 
>> to get his feedback.
>> 
>> On 4 May 2011, at 10:46, Dan Berindei wrote:
>> 
>>> On Wed, May 4, 2011 at 5:09 PM, Pete Muir  wrote:
 On 4 May 2011, at 05:34, Dan Berindei wrote:
 
> On Wed, May 4, 2011 at 10:14 AM, Galder Zamarreño  
> wrote:
>> On May 3, 2011, at 2:33 PM, Sanne Grinovero wrote:
>> 
>>> 2011/5/3 "이희승 (Trustin Lee)":
 On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
> 2011/5/2 Manik Surtani:
>> On 1 May 2011, at 13:38, Pete Muir wrote:
>> 
> As in, user API?  That's a little intrusive... e.g., put(K, 
> V, cl) ?
 Not for put, since you have the class, just get, and I was 
 thinking
 something more like:
 
>>

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

[Galder Zamarreño]

Guys,

It's funny that we're talking about this but this appears to be precisely the 
use case that we don't support as per user forum post in: 
http://community.jboss.org/thread/166080?tstart=0

Pete, do we have a JIRA for this? If not, would you mind creating one?

Cheers,

On May 9, 2011, at 5:37 PM, Pete Muir wrote:

> Ok, let me stew on that ;-)
> 
> On 9 May 2011, at 16:35, David Bosschaert wrote:
> 
>> If you have a Bundle object or BundleContext object you can figure out what 
>> classloader is associated with that. Also, if you have a class from a bundle 
>> you can find out what it's classloader is (obviously).
>> 
>> However, you need to have one of those things to find this out. There is 
>> nothing magically available in the current context to tell you what the 
>> current Bundle is.
>> It's generally really easy for the caller though to find out what the 
>> classloader is, though... If you have bundle X, you'd know a class from that 
>> bundle a.b.c.D (any class will do). Then you can simply call 
>> D.class.getClassLoader() and you've got the Bundle Classloader.
>> 
>> Hope this helps,
>> 
>> David
>> 
>> On 09/05/2011 16:27, Pete Muir wrote:
>>> The issue that David raises is that in an OSGi env that this wouldn't be 
>>> done by OSGi so would be up to the user or would require some extra 
>>> integration library. I'm not even sure if this is possible in OSGi? David, 
>>> is there anyway for a framework to "find out" the current bundle 
>>> classloader in OSGi rather than having to be told it (i.e. push rather than 
>>> pull)?
>>> 
>>> The idea is that in AS that by doing (a) it would require the integration 
>>> to make sure the TCCL is set before Infinispan is called (this is the way 
>>> many things are integrated into GF for example).
>>> 
>>> On 5 May 2011, at 20:05, Emmanuel Bernard wrote:
>>> 
 Quick question.
 In case of 2.a (ie setting the TCCL), this is a requirement for frameworks 
 and libraries using Infinispan, right? Not / never for a user application 
 using Infinispan (in this case it seems that the application class loader 
 will do the right thing and is set as the TCCL by the AS environment)?
 
 Emmanuel
 
 On 5 mai 2011, at 10:55, Pete Muir wrote:
 
> I talked about this with Emmanuel last night, and we were
> 
> a) concerned about the pollution of the API that this implies
> b) not sure why we need to do this
> 
> So I also spoke to Jason to understand his initial motivation, and from 
> this chat I think it's clear that things have gone off course here.
> 
> Jason raised two issues with modularity/classloading in Infinispan:
> 
> 1) Using the TCCL as the classloader to load Infinispan classes is a bad 
> idea. Instead we should use 
> org.infinispan.foo.Bar.getClass().getClassLoader().
> 
> 2) When we need to load application classes we need a different approach 
> to that used today. Most of the time the TCCL is perfect for this. 
> However *sometimes* Infinispan may be invoked outside of the application, 
> when the TCCL may not be set in AS7. Jason and I discussed three options:
> 
> a) require (through a platform integration documentation contract) that 
> the TCCL must always be set when Infinispan is invoked.
> b) Have some sort of InvocationContext which knows what the correct 
> classloader to use is (aka Hibernate/Seam/Weld design where there is a 
> per-application construct based on a ThreadLocal). Given this hasn't been 
> designed into the core, it seems like a large retrofit
> c) Make users specify the CL (directly or indirectly) via the API (as we 
> discussed).
> 
> Personally I think that (a) is the best approach right now, and is not 
> that onerous a requirement.
> 
> We might want to make the TCCL usage pluggable for OSGI envs. Cc'd David 
> to get his feedback.
> 
> On 4 May 2011, at 10:46, Dan Berindei wrote:
> 
>> On Wed, May 4, 2011 at 5:09 PM, Pete Muir  wrote:
>>> On 4 May 2011, at 05:34, Dan Berindei wrote:
>>> 
 On Wed, May 4, 2011 at 10:14 AM, Galder Zamarreño  
 wrote:
> On May 3, 2011, at 2:33 PM, Sanne Grinovero wrote:
> 
>> 2011/5/3 "이희승 (Trustin Lee)":
>>> On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
 2011/5/2 Manik Surtani:
> On 1 May 2011, at 13:38, Pete Muir wrote:
> 
 As in, user API?  That's a little intrusive... e.g., put(K, V, 
 cl) ?
>>> Not for put, since you have the class, just get, and I was 
>>> thinking
>>> something more like:
>>> 
>>> Foo foo = getUsing(key, Foo.class)
>> This would be a pretty useful addition to the API anyway to 
>> avoid user casts.
> Maybe as an "advanced" API, so as not to pollute the basic API

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

[Pete Muir]

Ok, let me stew on that ;-)

On 9 May 2011, at 16:35, David Bosschaert wrote:

> If you have a Bundle object or BundleContext object you can figure out what 
> classloader is associated with that. Also, if you have a class from a bundle 
> you can find out what it's classloader is (obviously).
> 
> However, you need to have one of those things to find this out. There is 
> nothing magically available in the current context to tell you what the 
> current Bundle is.
> It's generally really easy for the caller though to find out what the 
> classloader is, though... If you have bundle X, you'd know a class from that 
> bundle a.b.c.D (any class will do). Then you can simply call 
> D.class.getClassLoader() and you've got the Bundle Classloader.
> 
> Hope this helps,
> 
> David
> 
> On 09/05/2011 16:27, Pete Muir wrote:
>> The issue that David raises is that in an OSGi env that this wouldn't be 
>> done by OSGi so would be up to the user or would require some extra 
>> integration library. I'm not even sure if this is possible in OSGi? David, 
>> is there anyway for a framework to "find out" the current bundle classloader 
>> in OSGi rather than having to be told it (i.e. push rather than pull)?
>> 
>> The idea is that in AS that by doing (a) it would require the integration to 
>> make sure the TCCL is set before Infinispan is called (this is the way many 
>> things are integrated into GF for example).
>> 
>> On 5 May 2011, at 20:05, Emmanuel Bernard wrote:
>> 
>>> Quick question.
>>> In case of 2.a (ie setting the TCCL), this is a requirement for frameworks 
>>> and libraries using Infinispan, right? Not / never for a user application 
>>> using Infinispan (in this case it seems that the application class loader 
>>> will do the right thing and is set as the TCCL by the AS environment)?
>>> 
>>> Emmanuel
>>> 
>>> On 5 mai 2011, at 10:55, Pete Muir wrote:
>>> 
 I talked about this with Emmanuel last night, and we were
 
 a) concerned about the pollution of the API that this implies
 b) not sure why we need to do this
 
 So I also spoke to Jason to understand his initial motivation, and from 
 this chat I think it's clear that things have gone off course here.
 
 Jason raised two issues with modularity/classloading in Infinispan:
 
 1) Using the TCCL as the classloader to load Infinispan classes is a bad 
 idea. Instead we should use 
 org.infinispan.foo.Bar.getClass().getClassLoader().
 
 2) When we need to load application classes we need a different approach 
 to that used today. Most of the time the TCCL is perfect for this. However 
 *sometimes* Infinispan may be invoked outside of the application, when the 
 TCCL may not be set in AS7. Jason and I discussed three options:
 
 a) require (through a platform integration documentation contract) that 
 the TCCL must always be set when Infinispan is invoked.
 b) Have some sort of InvocationContext which knows what the correct 
 classloader to use is (aka Hibernate/Seam/Weld design where there is a 
 per-application construct based on a ThreadLocal). Given this hasn't been 
 designed into the core, it seems like a large retrofit
 c) Make users specify the CL (directly or indirectly) via the API (as we 
 discussed).
 
 Personally I think that (a) is the best approach right now, and is not 
 that onerous a requirement.
 
 We might want to make the TCCL usage pluggable for OSGI envs. Cc'd David 
 to get his feedback.
 
 On 4 May 2011, at 10:46, Dan Berindei wrote:
 
> On Wed, May 4, 2011 at 5:09 PM, Pete Muir  wrote:
>> On 4 May 2011, at 05:34, Dan Berindei wrote:
>> 
>>> On Wed, May 4, 2011 at 10:14 AM, Galder Zamarreño  
>>> wrote:
 On May 3, 2011, at 2:33 PM, Sanne Grinovero wrote:
 
> 2011/5/3 "이희승 (Trustin Lee)":
>> On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
>>> 2011/5/2 Manik Surtani:
 On 1 May 2011, at 13:38, Pete Muir wrote:
 
>>> As in, user API?  That's a little intrusive... e.g., put(K, V, 
>>> cl) ?
>> Not for put, since you have the class, just get, and I was 
>> thinking
>> something more like:
>> 
>> Foo foo = getUsing(key, Foo.class)
> This would be a pretty useful addition to the API anyway to avoid 
> user casts.
 Maybe as an "advanced" API, so as not to pollute the basic API?  A 
 bit like:
 
 Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);
>>> doesn't look much better than a cast, but is more cryptical :)
>>> 
>>> getting back to the classloader issue, what about:
>>> 
>>> Cache c = cacheManager.getCache( cacheName, classLoader );
>>> 
>>> or
>>> Cache c = cacheManager.getCache( cacheNa

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

[David Bosschaert]

If you have a Bundle object or BundleContext object you can figure out 
what classloader is associated with that. Also, if you have a class from 
a bundle you can find out what it's classloader is (obviously).

However, you need to have one of those things to find this out. There is 
nothing magically available in the current context to tell you what the 
current Bundle is.
It's generally really easy for the caller though to find out what the 
classloader is, though... If you have bundle X, you'd know a class from 
that bundle a.b.c.D (any class will do). Then you can simply call 
D.class.getClassLoader() and you've got the Bundle Classloader.

Hope this helps,

David

On 09/05/2011 16:27, Pete Muir wrote:
> The issue that David raises is that in an OSGi env that this wouldn't be done 
> by OSGi so would be up to the user or would require some extra integration 
> library. I'm not even sure if this is possible in OSGi? David, is there 
> anyway for a framework to "find out" the current bundle classloader in OSGi 
> rather than having to be told it (i.e. push rather than pull)?
>
> The idea is that in AS that by doing (a) it would require the integration to 
> make sure the TCCL is set before Infinispan is called (this is the way many 
> things are integrated into GF for example).
>
> On 5 May 2011, at 20:05, Emmanuel Bernard wrote:
>
>> Quick question.
>> In case of 2.a (ie setting the TCCL), this is a requirement for frameworks 
>> and libraries using Infinispan, right? Not / never for a user application 
>> using Infinispan (in this case it seems that the application class loader 
>> will do the right thing and is set as the TCCL by the AS environment)?
>>
>> Emmanuel
>>
>> On 5 mai 2011, at 10:55, Pete Muir wrote:
>>
>>> I talked about this with Emmanuel last night, and we were
>>>
>>> a) concerned about the pollution of the API that this implies
>>> b) not sure why we need to do this
>>>
>>> So I also spoke to Jason to understand his initial motivation, and from 
>>> this chat I think it's clear that things have gone off course here.
>>>
>>> Jason raised two issues with modularity/classloading in Infinispan:
>>>
>>> 1) Using the TCCL as the classloader to load Infinispan classes is a bad 
>>> idea. Instead we should use 
>>> org.infinispan.foo.Bar.getClass().getClassLoader().
>>>
>>> 2) When we need to load application classes we need a different approach to 
>>> that used today. Most of the time the TCCL is perfect for this. However 
>>> *sometimes* Infinispan may be invoked outside of the application, when the 
>>> TCCL may not be set in AS7. Jason and I discussed three options:
>>>
>>> a) require (through a platform integration documentation contract) that the 
>>> TCCL must always be set when Infinispan is invoked.
>>> b) Have some sort of InvocationContext which knows what the correct 
>>> classloader to use is (aka Hibernate/Seam/Weld design where there is a 
>>> per-application construct based on a ThreadLocal). Given this hasn't been 
>>> designed into the core, it seems like a large retrofit
>>> c) Make users specify the CL (directly or indirectly) via the API (as we 
>>> discussed).
>>>
>>> Personally I think that (a) is the best approach right now, and is not that 
>>> onerous a requirement.
>>>
>>> We might want to make the TCCL usage pluggable for OSGI envs. Cc'd David to 
>>> get his feedback.
>>>
>>> On 4 May 2011, at 10:46, Dan Berindei wrote:
>>>
 On Wed, May 4, 2011 at 5:09 PM, Pete Muir  wrote:
> On 4 May 2011, at 05:34, Dan Berindei wrote:
>
>> On Wed, May 4, 2011 at 10:14 AM, Galder Zamarreño  
>> wrote:
>>> On May 3, 2011, at 2:33 PM, Sanne Grinovero wrote:
>>>
 2011/5/3 "이희승 (Trustin Lee)":
> On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
>> 2011/5/2 Manik Surtani:
>>> On 1 May 2011, at 13:38, Pete Muir wrote:
>>>
>> As in, user API?  That's a little intrusive... e.g., put(K, V, 
>> cl) ?
> Not for put, since you have the class, just get, and I was 
> thinking
> something more like:
>
> Foo foo = getUsing(key, Foo.class)
 This would be a pretty useful addition to the API anyway to avoid 
 user casts.
>>> Maybe as an "advanced" API, so as not to pollute the basic API?  A 
>>> bit like:
>>>
>>> Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);
>> doesn't look much better than a cast, but is more cryptical :)
>>
>> getting back to the classloader issue, what about:
>>
>> Cache c = cacheManager.getCache( cacheName, classLoader );
>>
>> or
>> Cache c = cacheManager.getCache( cacheName 
>> ).usingClassLoader(classLoader );
>>
>> BTW if that's an issue on the API, maybe you should propose it to
>> JSR-107 as well ?
> We have a configurable Marshaller, right?  Then why 

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

[Pete Muir]

The issue that David raises is that in an OSGi env that this wouldn't be done 
by OSGi so would be up to the user or would require some extra integration 
library. I'm not even sure if this is possible in OSGi? David, is there anyway 
for a framework to "find out" the current bundle classloader in OSGi rather 
than having to be told it (i.e. push rather than pull)?

The idea is that in AS that by doing (a) it would require the integration to 
make sure the TCCL is set before Infinispan is called (this is the way many 
things are integrated into GF for example).

On 5 May 2011, at 20:05, Emmanuel Bernard wrote:

> Quick question.
> In case of 2.a (ie setting the TCCL), this is a requirement for frameworks 
> and libraries using Infinispan, right? Not / never for a user application 
> using Infinispan (in this case it seems that the application class loader 
> will do the right thing and is set as the TCCL by the AS environment)?
> 
> Emmanuel
> 
> On 5 mai 2011, at 10:55, Pete Muir wrote:
> 
>> I talked about this with Emmanuel last night, and we were
>> 
>> a) concerned about the pollution of the API that this implies
>> b) not sure why we need to do this
>> 
>> So I also spoke to Jason to understand his initial motivation, and from this 
>> chat I think it's clear that things have gone off course here.
>> 
>> Jason raised two issues with modularity/classloading in Infinispan:
>> 
>> 1) Using the TCCL as the classloader to load Infinispan classes is a bad 
>> idea. Instead we should use 
>> org.infinispan.foo.Bar.getClass().getClassLoader().
>> 
>> 2) When we need to load application classes we need a different approach to 
>> that used today. Most of the time the TCCL is perfect for this. However 
>> *sometimes* Infinispan may be invoked outside of the application, when the 
>> TCCL may not be set in AS7. Jason and I discussed three options:
>> 
>> a) require (through a platform integration documentation contract) that the 
>> TCCL must always be set when Infinispan is invoked.
>> b) Have some sort of InvocationContext which knows what the correct 
>> classloader to use is (aka Hibernate/Seam/Weld design where there is a 
>> per-application construct based on a ThreadLocal). Given this hasn't been 
>> designed into the core, it seems like a large retrofit
>> c) Make users specify the CL (directly or indirectly) via the API (as we 
>> discussed).
>> 
>> Personally I think that (a) is the best approach right now, and is not that 
>> onerous a requirement.
>> 
>> We might want to make the TCCL usage pluggable for OSGI envs. Cc'd David to 
>> get his feedback.
>> 
>> On 4 May 2011, at 10:46, Dan Berindei wrote:
>> 
>>> On Wed, May 4, 2011 at 5:09 PM, Pete Muir  wrote:
 
 On 4 May 2011, at 05:34, Dan Berindei wrote:
 
> On Wed, May 4, 2011 at 10:14 AM, Galder Zamarreño  
> wrote:
>> 
>> On May 3, 2011, at 2:33 PM, Sanne Grinovero wrote:
>> 
>>> 2011/5/3 "이희승 (Trustin Lee)" :
 On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
> 2011/5/2 Manik Surtani :
>> 
>> On 1 May 2011, at 13:38, Pete Muir wrote:
>> 
> As in, user API?  That's a little intrusive... e.g., put(K, V, 
> cl) ?
 
 Not for put, since you have the class, just get, and I was thinking
 something more like:
 
 Foo foo = getUsing(key, Foo.class)
>>> 
>>> This would be a pretty useful addition to the API anyway to avoid 
>>> user casts.
>> 
>> Maybe as an "advanced" API, so as not to pollute the basic API?  A 
>> bit like:
>> 
>> Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);
> 
> doesn't look much better than a cast, but is more cryptical :)
> 
> getting back to the classloader issue, what about:
> 
> Cache c = cacheManager.getCache( cacheName, classLoader );
> 
> or
> Cache c = cacheManager.getCache( cacheName 
> ).usingClassLoader(classLoader );
> 
> BTW if that's an issue on the API, maybe you should propose it to
> JSR-107 as well ?
 
 We have a configurable Marshaller, right?  Then why don't we just use
 the class loader that the current Marshaller uses?
>>> 
>>> +1
>>> I like the clean approach, not sure how you configure the "current
>>> Marshaller" to use the correct CL ?
>>> Likely hard to do via configuration file :)
>> 
>> Well, the marshaller is a global component and so it's a cache manager 
>> level. You can't make any assumptions about it's classloader, 
>> particularly when lazy deserialization is configured and you want to 
>> make sure that the data of the cache is deserialized with the correct 
>> classloader when the user reads the data from the cache. This is gonna 
>> become even more important when we for example move to hav

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

[Dan Berindei]

On Thu, May 5, 2011 at 10:05 PM, Emmanuel Bernard
 wrote:
> Quick question.
> In case of 2.a (ie setting the TCCL), this is a requirement for frameworks 
> and libraries using Infinispan, right? Not / never for a user application 
> using Infinispan (in this case it seems that the application class loader 
> will do the right thing and is set as the TCCL by the AS environment)?
>

My understanding is that we only need to worry about the TCCL if a
library is creating threads and is sharing those threads between
applications - when you create a new thread it inherits the TCCL from
it's parent thread. I'm not sure if we should expect the library to
set the TCCL around user callbacks in this case though, because the
TCCL is only necessary + guaranteed to work in a JEE environment: in a
standalone application you have basically one classloader, in an OSGi
application the TCCL is not guaranteed to be set properly anyway.

It's also possible that an application will want to store in the cache
an object whose class is not accessible through the application's
classloader.
E.g. if another module has an interface A and a factory for A objects
in an exported package that's accessible from the application's
classloader but the actual implementation class is not exported and so
it's not accessible.

For option 2.b we already have the InvocationContext, we only need to
add the classloader to it. Even if we would go the 2.a route, for
async calls I think we can unmarshal the object on a different thread
(which I also think could be shared between apps), so we may need to
save the TCCL at the time of the get() call in the InvocationContext
and use that for unmarshalling.

So I'd choose 2.b with TCCL as the default classloader. The only
catch, as Galder said, is we have to be very careful not to forget
invocation contexts lying around or we will leak classloaders.

Dan


>
> On 5 mai 2011, at 10:55, Pete Muir wrote:
>
>> I talked about this with Emmanuel last night, and we were
>>
>> a) concerned about the pollution of the API that this implies
>> b) not sure why we need to do this
>>
>> So I also spoke to Jason to understand his initial motivation, and from this 
>> chat I think it's clear that things have gone off course here.
>>
>> Jason raised two issues with modularity/classloading in Infinispan:
>>
>> 1) Using the TCCL as the classloader to load Infinispan classes is a bad 
>> idea. Instead we should use 
>> org.infinispan.foo.Bar.getClass().getClassLoader().
>>
>> 2) When we need to load application classes we need a different approach to 
>> that used today. Most of the time the TCCL is perfect for this. However 
>> *sometimes* Infinispan may be invoked outside of the application, when the 
>> TCCL may not be set in AS7. Jason and I discussed three options:
>>
>> a) require (through a platform integration documentation contract) that the 
>> TCCL must always be set when Infinispan is invoked.
>> b) Have some sort of InvocationContext which knows what the correct 
>> classloader to use is (aka Hibernate/Seam/Weld design where there is a 
>> per-application construct based on a ThreadLocal). Given this hasn't been 
>> designed into the core, it seems like a large retrofit
>> c) Make users specify the CL (directly or indirectly) via the API (as we 
>> discussed).
>>
>> Personally I think that (a) is the best approach right now, and is not that 
>> onerous a requirement.
>>
>> We might want to make the TCCL usage pluggable for OSGI envs. Cc'd David to 
>> get his feedback.
>>
>> On 4 May 2011, at 10:46, Dan Berindei wrote:
>>
>>> On Wed, May 4, 2011 at 5:09 PM, Pete Muir  wrote:

 On 4 May 2011, at 05:34, Dan Berindei wrote:

> On Wed, May 4, 2011 at 10:14 AM, Galder Zamarreño  
> wrote:
>>
>> On May 3, 2011, at 2:33 PM, Sanne Grinovero wrote:
>>
>>> 2011/5/3 "이희승 (Trustin Lee)" :
 On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
> 2011/5/2 Manik Surtani :
>>
>> On 1 May 2011, at 13:38, Pete Muir wrote:
>>
> As in, user API?  That's a little intrusive... e.g., put(K, V, 
> cl) ?

 Not for put, since you have the class, just get, and I was thinking
 something more like:

 Foo foo = getUsing(key, Foo.class)
>>>
>>> This would be a pretty useful addition to the API anyway to avoid 
>>> user casts.
>>
>> Maybe as an "advanced" API, so as not to pollute the basic API?  A 
>> bit like:
>>
>> Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);
>
> doesn't look much better than a cast, but is more cryptical :)
>
> getting back to the classloader issue, what about:
>
> Cache c = cacheManager.getCache( cacheName, classLoader );
>
> or
> Cache c = cacheManager.getCache( cacheName 
> ).usingClassLoader(classLoader );

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

[Emmanuel Bernard]

Quick question.
In case of 2.a (ie setting the TCCL), this is a requirement for frameworks and 
libraries using Infinispan, right? Not / never for a user application using 
Infinispan (in this case it seems that the application class loader will do the 
right thing and is set as the TCCL by the AS environment)?

Emmanuel

On 5 mai 2011, at 10:55, Pete Muir wrote:

> I talked about this with Emmanuel last night, and we were
> 
> a) concerned about the pollution of the API that this implies
> b) not sure why we need to do this
> 
> So I also spoke to Jason to understand his initial motivation, and from this 
> chat I think it's clear that things have gone off course here.
> 
> Jason raised two issues with modularity/classloading in Infinispan:
> 
> 1) Using the TCCL as the classloader to load Infinispan classes is a bad 
> idea. Instead we should use 
> org.infinispan.foo.Bar.getClass().getClassLoader().
> 
> 2) When we need to load application classes we need a different approach to 
> that used today. Most of the time the TCCL is perfect for this. However 
> *sometimes* Infinispan may be invoked outside of the application, when the 
> TCCL may not be set in AS7. Jason and I discussed three options:
> 
> a) require (through a platform integration documentation contract) that the 
> TCCL must always be set when Infinispan is invoked.
> b) Have some sort of InvocationContext which knows what the correct 
> classloader to use is (aka Hibernate/Seam/Weld design where there is a 
> per-application construct based on a ThreadLocal). Given this hasn't been 
> designed into the core, it seems like a large retrofit
> c) Make users specify the CL (directly or indirectly) via the API (as we 
> discussed).
> 
> Personally I think that (a) is the best approach right now, and is not that 
> onerous a requirement.
> 
> We might want to make the TCCL usage pluggable for OSGI envs. Cc'd David to 
> get his feedback.
> 
> On 4 May 2011, at 10:46, Dan Berindei wrote:
> 
>> On Wed, May 4, 2011 at 5:09 PM, Pete Muir  wrote:
>>> 
>>> On 4 May 2011, at 05:34, Dan Berindei wrote:
>>> 
 On Wed, May 4, 2011 at 10:14 AM, Galder Zamarreño  
 wrote:
> 
> On May 3, 2011, at 2:33 PM, Sanne Grinovero wrote:
> 
>> 2011/5/3 "이희승 (Trustin Lee)" :
>>> On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
 2011/5/2 Manik Surtani :
> 
> On 1 May 2011, at 13:38, Pete Muir wrote:
> 
 As in, user API?  That's a little intrusive... e.g., put(K, V, cl) 
 ?
>>> 
>>> Not for put, since you have the class, just get, and I was thinking
>>> something more like:
>>> 
>>> Foo foo = getUsing(key, Foo.class)
>> 
>> This would be a pretty useful addition to the API anyway to avoid 
>> user casts.
> 
> Maybe as an "advanced" API, so as not to pollute the basic API?  A 
> bit like:
> 
> Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);
 
 doesn't look much better than a cast, but is more cryptical :)
 
 getting back to the classloader issue, what about:
 
 Cache c = cacheManager.getCache( cacheName, classLoader );
 
 or
 Cache c = cacheManager.getCache( cacheName 
 ).usingClassLoader(classLoader );
 
 BTW if that's an issue on the API, maybe you should propose it to
 JSR-107 as well ?
>>> 
>>> We have a configurable Marshaller, right?  Then why don't we just use
>>> the class loader that the current Marshaller uses?
>> 
>> +1
>> I like the clean approach, not sure how you configure the "current
>> Marshaller" to use the correct CL ?
>> Likely hard to do via configuration file :)
> 
> Well, the marshaller is a global component and so it's a cache manager 
> level. You can't make any assumptions about it's classloader, 
> particularly when lazy deserialization is configured and you want to make 
> sure that the data of the cache is deserialized with the correct 
> classloader when the user reads the data from the cache. This is gonna 
> become even more important when we for example move to having a single 
> cache for all 2LC entities or all EJB3 SFSBs where we'll definitely need 
> multiple classloaders to access a single cache.
> 
 
 The current unmarshaller uses the TCCL, which is a great idea for
 non-modular environments and will still work in AS7 for application
 classes (so it's still a good default). It probably won't work if
 Hibernate wants to store its own classes in the cache, because
 Hibernate's internal classes may not be reachable from the
 application's classloader.
 
 It gets even trickier if Hibernate wants to store a
 PrivateHibernateCollection class in the cache containing instances of
 application classes inside. Then I don't think the

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

[David M. Lloyd]

On 05/05/2011 10:49 AM, David Bosschaert wrote:
> On 05/05/2011 10:55, Pete Muir wrote:
>> I talked about this with Emmanuel last night, and we were
>>
>> a) concerned about the pollution of the API that this implies
>> b) not sure why we need to do this
>>
>> So I also spoke to Jason to understand his initial motivation, and from this 
>> chat I think it's clear that things have gone off course here.
>>
>> Jason raised two issues with modularity/classloading in Infinispan:
>>
>> 1) Using the TCCL as the classloader to load Infinispan classes is a bad 
>> idea. Instead we should use 
>> org.infinispan.foo.Bar.getClass().getClassLoader().
>>
>> 2) When we need to load application classes we need a different approach to 
>> that used today. Most of the time the TCCL is perfect for this. However 
>> *sometimes* Infinispan may be invoked outside of the application, when the 
>> TCCL may not be set in AS7. Jason and I discussed three options:
>>
>> a) require (through a platform integration documentation contract) that the 
>> TCCL must always be set when Infinispan is invoked.
>> b) Have some sort of InvocationContext which knows what the correct 
>> classloader to use is (aka Hibernate/Seam/Weld design where there is a 
>> per-application construct based on a ThreadLocal). Given this hasn't been 
>> designed into the core, it seems like a large retrofit
>> c) Make users specify the CL (directly or indirectly) via the API (as we 
>> discussed).
>>
>> Personally I think that (a) is the best approach right now, and is not that 
>> onerous a requirement.
>>
>> We might want to make the TCCL usage pluggable for OSGI envs. Cc'd David to 
>> get his feedback.
>
> In a sense a) and c) are similar. The only difference is the way you
> provide the classloader.
>
> OSGi doesn't specify the value of the Thread Context Classloader,
> primarily because it allows the creation of threads in your own code so
> OSGi can't control the value of the TCCL in those threads.
>
> a) Would be okay for OSGi as long as it's clearly documented. People
> would have to write code to set the TCCL before they call infinispan and
> then unset if afterwards. It requires a try/finally block which isn't
> fun to write and its a little verbose, but you can do it.

I don't know if OSGi is compatible with security managers, but this also 
has the added annoyance of potentially requiring privileged blocks in 
this case.

> c) Would be more user-friendly to OSGi users. It's easy to obtain the
> classloader of a bundle in OSGi so passing that in would be nice and easy.
>
> I guess you could consider providing both a) and c) at the same time, c)
> being simply an additional overload on the APIs so that people can
> choose whichever option they like best...

I like (c) better as well - I agree, doing both would be nicest.
-- 
- DML
___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[David Bosschaert]

On 05/05/2011 10:55, Pete Muir wrote:
> I talked about this with Emmanuel last night, and we were
>
> a) concerned about the pollution of the API that this implies
> b) not sure why we need to do this
>
> So I also spoke to Jason to understand his initial motivation, and from this 
> chat I think it's clear that things have gone off course here.
>
> Jason raised two issues with modularity/classloading in Infinispan:
>
> 1) Using the TCCL as the classloader to load Infinispan classes is a bad 
> idea. Instead we should use 
> org.infinispan.foo.Bar.getClass().getClassLoader().
>
> 2) When we need to load application classes we need a different approach to 
> that used today. Most of the time the TCCL is perfect for this. However 
> *sometimes* Infinispan may be invoked outside of the application, when the 
> TCCL may not be set in AS7. Jason and I discussed three options:
>
> a) require (through a platform integration documentation contract) that the 
> TCCL must always be set when Infinispan is invoked.
> b) Have some sort of InvocationContext which knows what the correct 
> classloader to use is (aka Hibernate/Seam/Weld design where there is a 
> per-application construct based on a ThreadLocal). Given this hasn't been 
> designed into the core, it seems like a large retrofit
> c) Make users specify the CL (directly or indirectly) via the API (as we 
> discussed).
>
> Personally I think that (a) is the best approach right now, and is not that 
> onerous a requirement.
>
> We might want to make the TCCL usage pluggable for OSGI envs. Cc'd David to 
> get his feedback.

In a sense a) and c) are similar. The only difference is the way you 
provide the classloader.

OSGi doesn't specify the value of the Thread Context Classloader, 
primarily because it allows the creation of threads in your own code so 
OSGi can't control the value of the TCCL in those threads.

a) Would be okay for OSGi as long as it's clearly documented. People 
would have to write code to set the TCCL before they call infinispan and 
then unset if afterwards. It requires a try/finally block which isn't 
fun to write and its a little verbose, but you can do it.

c) Would be more user-friendly to OSGi users. It's easy to obtain the 
classloader of a bundle in OSGi so passing that in would be nice and easy.

I guess you could consider providing both a) and c) at the same time, c) 
being simply an additional overload on the APIs so that people can 
choose whichever option they like best...

Cheers,

David
___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Pete Muir]

On 5 May 2011, at 03:31, Galder Zamarreño wrote:

> 
> On May 4, 2011, at 4:08 PM, Pete Muir wrote:
> 
>> 
>> On 4 May 2011, at 09:55, Dan Berindei wrote:
>> 
>>> On Wed, May 4, 2011 at 7:18 AM, "이희승 (Trustin Lee)"  
>>> wrote:
 On 05/03/2011 09:33 PM, Sanne Grinovero wrote:
> 2011/5/3 "이희승 (Trustin Lee)" :
>> On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
>>> 2011/5/2 Manik Surtani :
 
 On 1 May 2011, at 13:38, Pete Muir wrote:
 
>>> As in, user API?  That's a little intrusive... e.g., put(K, V, cl) ?
>> 
>> Not for put, since you have the class, just get, and I was thinking
>> something more like:
>> 
>> Foo foo = getUsing(key, Foo.class)
> 
> This would be a pretty useful addition to the API anyway to avoid 
> user casts.
 
 Maybe as an "advanced" API, so as not to pollute the basic API?  A bit 
 like:
 
 Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);
>>> 
>>> doesn't look much better than a cast, but is more cryptical :)
>>> 
>>> getting back to the classloader issue, what about:
>>> 
>>> Cache c = cacheManager.getCache( cacheName, classLoader );
>>> 
>>> or
>>> Cache c = cacheManager.getCache( cacheName 
>>> ).usingClassLoader(classLoader );
>>> 
>>> BTW if that's an issue on the API, maybe you should propose it to
>>> JSR-107 as well ?
>> 
>> We have a configurable Marshaller, right?  Then why don't we just use
>> the class loader that the current Marshaller uses?
> 
> +1
> I like the clean approach, not sure how you configure the "current
> Marshaller" to use the correct CL ?
> Likely hard to do via configuration file :)
 
 By default, we could use the class loader that the current Unmarshaller
 uses, and let user choose a class loader for a certain get() call.
 
 So, we need to deal with the two issues here:
 
 1) Make sure that user can configure the default class loader in runtime
 and calling get() (and consequently related unmarshaller) will use the
 default class loader:
 
 cache.setDefaultClassLoader(UserClass.class.getClassLoader())
 
 2) Provide an additional API that allows a user to specify a class
 loader for the current transaction or context:
 
 cache.get(K key, Class clazz) // +1 to Pete's suggestion
 
>>> 
>>> If V is Set then Set.class.getClassLoader() will
>>> give you the system classloader, which in most cases won't be able the
>>> MyObject class. It may not even be a Set of course, it could
>>> be just Set.
>>> 
>>> We could have a "shortcut" so the users can pass in a class instead of
>>> a classloader to avoid writing ".getClassLoader()", but we shouldn't
>>> require any relation between the class passed in and the class of the
>>> return value.
>> 
>> There are two forces at play here. Firstly the desire to introduce greater 
>> type safety into the Cache API, by returning a type for user, rather than 
>> just an object, and secondly the need to specify the CL.
>> 
>> We could have an API like:
>> 
>>  V get(Object key, Class classLoaderType);
>> 
>> and an overloaded form
>> 
>>  V get(Object key, ClassLoader cl);
>> 
>> This does involve Infinispan having to do a runtime cast to V without the 
>> class type passed in as a parameter, but this may be a necessary evil in the 
>> interest of improving the user api.
>> 
>> An alternative which is somewhat more verbose, somewhat safer, and somewhat 
>> more complex to explain (but ultimately more logical):
>> 
>> // Use the classloader of the expectedType, this would be the normal case
>>  V get(Object key, Class expectedType);
>> 
>> // Specify the classloader to use as a class
>>  V get(Object key, Class expectedType, Class classLoader);
>> 
>> // Specify the classloader to use as a classloader
>>  V get(Object key, Class expectedType, ClassLoader classLoader);
>> 
>> We could also include
>> 
>> // Use the TCCL
>> V get(Object key);
>> 
>> Any thoughts?
> 
> There's something that does not convince about adding these classloader and 
> class params to the get() calls and that's the fact that you're not only 
> gonna have to do it for get(), but for all methods that return V and that is: 
> all get variants, getAsync...etc, and all the methods that return the 
> previous value, such as put, putAsync, replace, replaceAsyncetc and 
> that's a fair amount of extra methods. I much prefer an API like we used with 
> flags which has been partly suggested above, i.e.
> 
> cache.withClassloader(classloader).asClass(expectedType).get(Object key)
> cache.withClassloader(classloader).get(Object key)
> cache.asClass(expectedType).get(Object key)

In this case I would prefer to just do a cast :-)

See other email about resetting the discussion.
___
infinispan-dev mailing list
inf

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

[Pete Muir]

I talked about this with Emmanuel last night, and we were

a) concerned about the pollution of the API that this implies
b) not sure why we need to do this

So I also spoke to Jason to understand his initial motivation, and from this 
chat I think it's clear that things have gone off course here.

Jason raised two issues with modularity/classloading in Infinispan:

1) Using the TCCL as the classloader to load Infinispan classes is a bad idea. 
Instead we should use org.infinispan.foo.Bar.getClass().getClassLoader().

2) When we need to load application classes we need a different approach to 
that used today. Most of the time the TCCL is perfect for this. However 
*sometimes* Infinispan may be invoked outside of the application, when the TCCL 
may not be set in AS7. Jason and I discussed three options:

a) require (through a platform integration documentation contract) that the 
TCCL must always be set when Infinispan is invoked.
b) Have some sort of InvocationContext which knows what the correct classloader 
to use is (aka Hibernate/Seam/Weld design where there is a per-application 
construct based on a ThreadLocal). Given this hasn't been designed into the 
core, it seems like a large retrofit
c) Make users specify the CL (directly or indirectly) via the API (as we 
discussed).

Personally I think that (a) is the best approach right now, and is not that 
onerous a requirement.

We might want to make the TCCL usage pluggable for OSGI envs. Cc'd David to get 
his feedback.

On 4 May 2011, at 10:46, Dan Berindei wrote:

> On Wed, May 4, 2011 at 5:09 PM, Pete Muir  wrote:
>> 
>> On 4 May 2011, at 05:34, Dan Berindei wrote:
>> 
>>> On Wed, May 4, 2011 at 10:14 AM, Galder Zamarreño  wrote:
 
 On May 3, 2011, at 2:33 PM, Sanne Grinovero wrote:
 
> 2011/5/3 "이희승 (Trustin Lee)" :
>> On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
>>> 2011/5/2 Manik Surtani :
 
 On 1 May 2011, at 13:38, Pete Muir wrote:
 
>>> As in, user API?  That's a little intrusive... e.g., put(K, V, cl) ?
>> 
>> Not for put, since you have the class, just get, and I was thinking
>> something more like:
>> 
>> Foo foo = getUsing(key, Foo.class)
> 
> This would be a pretty useful addition to the API anyway to avoid 
> user casts.
 
 Maybe as an "advanced" API, so as not to pollute the basic API?  A bit 
 like:
 
 Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);
>>> 
>>> doesn't look much better than a cast, but is more cryptical :)
>>> 
>>> getting back to the classloader issue, what about:
>>> 
>>> Cache c = cacheManager.getCache( cacheName, classLoader );
>>> 
>>> or
>>> Cache c = cacheManager.getCache( cacheName 
>>> ).usingClassLoader(classLoader );
>>> 
>>> BTW if that's an issue on the API, maybe you should propose it to
>>> JSR-107 as well ?
>> 
>> We have a configurable Marshaller, right?  Then why don't we just use
>> the class loader that the current Marshaller uses?
> 
> +1
> I like the clean approach, not sure how you configure the "current
> Marshaller" to use the correct CL ?
> Likely hard to do via configuration file :)
 
 Well, the marshaller is a global component and so it's a cache manager 
 level. You can't make any assumptions about it's classloader, particularly 
 when lazy deserialization is configured and you want to make sure that the 
 data of the cache is deserialized with the correct classloader when the 
 user reads the data from the cache. This is gonna become even more 
 important when we for example move to having a single cache for all 2LC 
 entities or all EJB3 SFSBs where we'll definitely need multiple 
 classloaders to access a single cache.
 
>>> 
>>> The current unmarshaller uses the TCCL, which is a great idea for
>>> non-modular environments and will still work in AS7 for application
>>> classes (so it's still a good default). It probably won't work if
>>> Hibernate wants to store its own classes in the cache, because
>>> Hibernate's internal classes may not be reachable from the
>>> application's classloader.
>>> 
>>> It gets even trickier if Hibernate wants to store a
>>> PrivateHibernateCollection class in the cache containing instances of
>>> application classes inside. Then I don't think there will be any
>>> single classloader that could reach both the Hibernate classes and the
>>> application classes so it can properly unmarshal both. Perhaps that's
>>> just something for the Hibernate folks to worry about? Or maybe we
>>> should allow the users to register more than one classloader with a
>>> cache?
>> 
>> You need to use a bridge classloader in this case.
> 
> You're right of course, Hibernate must have received/guessed the
> application's classloader and so it is able to create a bridge
> classloader that 

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

[Galder Zamarreño]

On May 4, 2011, at 4:08 PM, Pete Muir wrote:

> 
> On 4 May 2011, at 09:55, Dan Berindei wrote:
> 
>> On Wed, May 4, 2011 at 7:18 AM, "이희승 (Trustin Lee)"  
>> wrote:
>>> On 05/03/2011 09:33 PM, Sanne Grinovero wrote:
 2011/5/3 "이희승 (Trustin Lee)" :
> On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
>> 2011/5/2 Manik Surtani :
>>> 
>>> On 1 May 2011, at 13:38, Pete Muir wrote:
>>> 
>> As in, user API?  That's a little intrusive... e.g., put(K, V, cl) ?
> 
> Not for put, since you have the class, just get, and I was thinking
> something more like:
> 
> Foo foo = getUsing(key, Foo.class)
 
 This would be a pretty useful addition to the API anyway to avoid user 
 casts.
>>> 
>>> Maybe as an "advanced" API, so as not to pollute the basic API?  A bit 
>>> like:
>>> 
>>> Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);
>> 
>> doesn't look much better than a cast, but is more cryptical :)
>> 
>> getting back to the classloader issue, what about:
>> 
>> Cache c = cacheManager.getCache( cacheName, classLoader );
>> 
>> or
>> Cache c = cacheManager.getCache( cacheName 
>> ).usingClassLoader(classLoader );
>> 
>> BTW if that's an issue on the API, maybe you should propose it to
>> JSR-107 as well ?
> 
> We have a configurable Marshaller, right?  Then why don't we just use
> the class loader that the current Marshaller uses?
 
 +1
 I like the clean approach, not sure how you configure the "current
 Marshaller" to use the correct CL ?
 Likely hard to do via configuration file :)
>>> 
>>> By default, we could use the class loader that the current Unmarshaller
>>> uses, and let user choose a class loader for a certain get() call.
>>> 
>>> So, we need to deal with the two issues here:
>>> 
>>> 1) Make sure that user can configure the default class loader in runtime
>>> and calling get() (and consequently related unmarshaller) will use the
>>> default class loader:
>>> 
>>> cache.setDefaultClassLoader(UserClass.class.getClassLoader())
>>> 
>>> 2) Provide an additional API that allows a user to specify a class
>>> loader for the current transaction or context:
>>> 
>>> cache.get(K key, Class clazz) // +1 to Pete's suggestion
>>> 
>> 
>> If V is Set then Set.class.getClassLoader() will
>> give you the system classloader, which in most cases won't be able the
>> MyObject class. It may not even be a Set of course, it could
>> be just Set.
>> 
>> We could have a "shortcut" so the users can pass in a class instead of
>> a classloader to avoid writing ".getClassLoader()", but we shouldn't
>> require any relation between the class passed in and the class of the
>> return value.
> 
> There are two forces at play here. Firstly the desire to introduce greater 
> type safety into the Cache API, by returning a type for user, rather than 
> just an object, and secondly the need to specify the CL.
> 
> We could have an API like:
> 
>  V get(Object key, Class classLoaderType);
> 
> and an overloaded form
> 
>  V get(Object key, ClassLoader cl);
> 
> This does involve Infinispan having to do a runtime cast to V without the 
> class type passed in as a parameter, but this may be a necessary evil in the 
> interest of improving the user api.
> 
> An alternative which is somewhat more verbose, somewhat safer, and somewhat 
> more complex to explain (but ultimately more logical):
> 
> // Use the classloader of the expectedType, this would be the normal case
>  V get(Object key, Class expectedType);
> 
> // Specify the classloader to use as a class
>  V get(Object key, Class expectedType, Class classLoader);
> 
> // Specify the classloader to use as a classloader
>  V get(Object key, Class expectedType, ClassLoader classLoader);
> 
> We could also include
> 
> // Use the TCCL
> V get(Object key);
> 
> Any thoughts?

There's something that does not convince about adding these classloader and 
class params to the get() calls and that's the fact that you're not only gonna 
have to do it for get(), but for all methods that return V and that is: all get 
variants, getAsync...etc, and all the methods that return the previous value, 
such as put, putAsync, replace, replaceAsyncetc and that's a fair amount of 
extra methods. I much prefer an API like we used with flags which has been 
partly suggested above, i.e.

cache.withClassloader(classloader).asClass(expectedType).get(Object key)
cache.withClassloader(classloader).get(Object key)
cache.asClass(expectedType).get(Object key)

This would however require the classloader to be set as a thread local which 
makes me a panic a bit ;)

Cheers,
--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Dan Berindei]

2011/5/4 Pete Muir :
>
> On 4 May 2011, at 09:55, Dan Berindei wrote:
>
>> On Wed, May 4, 2011 at 7:18 AM, "이희승 (Trustin Lee)"  
>> wrote:
>>> On 05/03/2011 09:33 PM, Sanne Grinovero wrote:
 2011/5/3 "이희승 (Trustin Lee)" :
> On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
>> 2011/5/2 Manik Surtani :
>>>
>>> On 1 May 2011, at 13:38, Pete Muir wrote:
>>>
>> As in, user API?  That's a little intrusive... e.g., put(K, V, cl) ?
>
> Not for put, since you have the class, just get, and I was thinking
> something more like:
>
> Foo foo = getUsing(key, Foo.class)

 This would be a pretty useful addition to the API anyway to avoid user 
 casts.
>>>
>>> Maybe as an "advanced" API, so as not to pollute the basic API?  A bit 
>>> like:
>>>
>>> Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);
>>
>> doesn't look much better than a cast, but is more cryptical :)
>>
>> getting back to the classloader issue, what about:
>>
>> Cache c = cacheManager.getCache( cacheName, classLoader );
>>
>> or
>> Cache c = cacheManager.getCache( cacheName 
>> ).usingClassLoader(classLoader );
>>
>> BTW if that's an issue on the API, maybe you should propose it to
>> JSR-107 as well ?
>
> We have a configurable Marshaller, right?  Then why don't we just use
> the class loader that the current Marshaller uses?

 +1
 I like the clean approach, not sure how you configure the "current
 Marshaller" to use the correct CL ?
 Likely hard to do via configuration file :)
>>>
>>> By default, we could use the class loader that the current Unmarshaller
>>> uses, and let user choose a class loader for a certain get() call.
>>>
>>> So, we need to deal with the two issues here:
>>>
>>> 1) Make sure that user can configure the default class loader in runtime
>>> and calling get() (and consequently related unmarshaller) will use the
>>> default class loader:
>>>
>>>  cache.setDefaultClassLoader(UserClass.class.getClassLoader())
>>>
>>> 2) Provide an additional API that allows a user to specify a class
>>> loader for the current transaction or context:
>>>
>>>  cache.get(K key, Class clazz) // +1 to Pete's suggestion
>>>
>>
>> If V is Set then Set.class.getClassLoader() will
>> give you the system classloader, which in most cases won't be able the
>> MyObject class. It may not even be a Set of course, it could
>> be just Set.
>>
>> We could have a "shortcut" so the users can pass in a class instead of
>> a classloader to avoid writing ".getClassLoader()", but we shouldn't
>> require any relation between the class passed in and the class of the
>> return value.
>
> There are two forces at play here. Firstly the desire to introduce greater 
> type safety into the Cache API, by returning a type for user, rather than 
> just an object, and secondly the need to specify the CL.
>
> We could have an API like:
>
>  V get(Object key, Class classLoaderType);
>
> and an overloaded form
>
>  V get(Object key, ClassLoader cl);
>
> This does involve Infinispan having to do a runtime cast to V without the 
> class type passed in as a parameter, but this may be a necessary evil in the 
> interest of improving the user api.
>

Cache extends Map, so we already have runtime casts to V for

V get(Object key)

It seems natural to extend it to also return objects of the cache's V
type parameter instead of returning an arbitrary type:

 V get(Object key, Class classLoaderType);
V get(Object key, ClassLoader cl);

Besides, allowing get() to return an arbitrary type will only make the
user code look more type safe, without changing its behaviour at all.
If the user wants type safety he can already define different caches
with different V type parameters and in that case we will actually
enforce that the put argument is assignable to V at compile time.

> An alternative which is somewhat more verbose, somewhat safer, and somewhat 
> more complex to explain (but ultimately more logical):
>
> // Use the classloader of the expectedType, this would be the normal case
>  V get(Object key, Class expectedType);
>
> // Specify the classloader to use as a class
>  V get(Object key, Class expectedType, Class classLoader);
>
> // Specify the classloader to use as a classloader
>  V get(Object key, Class expectedType, ClassLoader classLoader);
>
> We could also include
>
> // Use the TCCL
> V get(Object key);
>
> Any thoughts?
>

I'd allow the user to set another default classloader on the cache
instead of the TCCL, like in Sanne's proposal:

Cache c = cacheManager.getCache( cacheName, defaultClassLoader );
or
Cache c = cacheManager.getCache( cacheName ).usingDefaultClassLoader(
classLoader );

Dan

___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[David M. Lloyd]

On 05/04/2011 09:08 AM, Pete Muir wrote:
> 
> On 4 May 2011, at 09:55, Dan Berindei wrote:
> 
>> On Wed, May 4, 2011 at 7:18 AM, "이희승 (Trustin
>> Lee)"  wrote:
>>> On 05/03/2011 09:33 PM, Sanne Grinovero wrote:
 2011/5/3 "이희승 (Trustin Lee)":
> On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
>> 2011/5/2 Manik Surtani:
>>> 
>>> On 1 May 2011, at 13:38, Pete Muir wrote:
>>> 
>> As in, user API?  That's a little intrusive...
>> e.g., put(K, V, cl) ?
> 
> Not for put, since you have the class, just get, and
> I was thinking something more like:
> 
> Foo foo = getUsing(key, Foo.class)
 
 This would be a pretty useful addition to the API
 anyway to avoid user casts.
>>> 
>>> Maybe as an "advanced" API, so as not to pollute the
>>> basic API?  A bit like:
>>> 
>>> Foo f =
>>> cache.getAdvancedCache().asClass(Foo.class).get(key);
>> 
>> doesn't look much better than a cast, but is more cryptical
>> :)
>> 
>> getting back to the classloader issue, what about:
>> 
>> Cache c = cacheManager.getCache( cacheName, classLoader );
>> 
>> or Cache c = cacheManager.getCache( cacheName
>> ).usingClassLoader(classLoader );
>> 
>> BTW if that's an issue on the API, maybe you should propose
>> it to JSR-107 as well ?
> 
> We have a configurable Marshaller, right?  Then why don't we
> just use the class loader that the current Marshaller uses?
 
 +1 I like the clean approach, not sure how you configure the
 "current Marshaller" to use the correct CL ? Likely hard to do
 via configuration file :)
>>> 
>>> By default, we could use the class loader that the current
>>> Unmarshaller uses, and let user choose a class loader for a
>>> certain get() call.
>>> 
>>> So, we need to deal with the two issues here:
>>> 
>>> 1) Make sure that user can configure the default class loader in
>>> runtime and calling get() (and consequently related unmarshaller)
>>> will use the default class loader:
>>> 
>>> cache.setDefaultClassLoader(UserClass.class.getClassLoader())
>>> 
>>> 2) Provide an additional API that allows a user to specify a
>>> class loader for the current transaction or context:
>>> 
>>> cache.get(K key, Class  clazz) // +1 to Pete's suggestion
>>> 
>> 
>> If V is Set  then Set.class.getClassLoader()
>> will give you the system classloader, which in most cases won't be
>> able the MyObject class. It may not even be a Set  of
>> course, it could be just Set.
>> 
>> We could have a "shortcut" so the users can pass in a class instead
>> of a classloader to avoid writing ".getClassLoader()", but we
>> shouldn't require any relation between the class passed in and the
>> class of the return value.
> 
> There are two forces at play here. Firstly the desire to introduce
> greater type safety into the Cache API, by returning a type for user,
> rather than just an object, and secondly the need to specify the CL.
> 
> We could have an API like:
> 
>   V get(Object key, Class  classLoaderType);

See note below about the CL type parameter.

> and an overloaded form
> 
>   V get(Object key, ClassLoader cl);
> 
> This does involve Infinispan having to do a runtime cast to V without
> the class type passed in as a parameter, but this may be a necessary
> evil in the interest of improving the user api.

There's no reason to do the runtime cast.  This should be spelled:

Object get(Object key, ClassLoader cl);

That way if there's a CCE, the user will stand a better chance of
understanding it (because hey, they did a cast).

> An alternative which is somewhat more verbose, somewhat safer, and
> somewhat more complex to explain (but ultimately more logical):
> 
> // Use the classloader of the expectedType, this would be the normal
> case   V get(Object key, Class  expectedType);
> 
> // Specify the classloader to use as a class
>   V get(Object key, Class  expectedType, Class
classLoader);

This one doesn't make a lot of sense to me.  In particular there's no
reason to have a type parameter CL unless you give it a bound in
relationship to V, in which case you could have just used the class
loader class in the first place.  Maybe you meant:

 V get(Object key, Class expected, Class
classWhoseClassLoaderShouldBeUsed);

And I would name that parameter exactly thus :)

> // Specify the classloader to use as a classloader 
>   V get(Object key, Class  expectedType, ClassLoader classLoader);
> 
> We could also include
> 
> // Use the TCCL
> V get(Object key);

Or a preset default CL?

-- 
- DML
___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Dan Berindei]

On Wed, May 4, 2011 at 5:09 PM, Pete Muir  wrote:
>
> On 4 May 2011, at 05:34, Dan Berindei wrote:
>
>> On Wed, May 4, 2011 at 10:14 AM, Galder Zamarreño  wrote:
>>>
>>> On May 3, 2011, at 2:33 PM, Sanne Grinovero wrote:
>>>
 2011/5/3 "이희승 (Trustin Lee)" :
> On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
>> 2011/5/2 Manik Surtani :
>>>
>>> On 1 May 2011, at 13:38, Pete Muir wrote:
>>>
>> As in, user API?  That's a little intrusive... e.g., put(K, V, cl) ?
>
> Not for put, since you have the class, just get, and I was thinking
> something more like:
>
> Foo foo = getUsing(key, Foo.class)

 This would be a pretty useful addition to the API anyway to avoid user 
 casts.
>>>
>>> Maybe as an "advanced" API, so as not to pollute the basic API?  A bit 
>>> like:
>>>
>>> Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);
>>
>> doesn't look much better than a cast, but is more cryptical :)
>>
>> getting back to the classloader issue, what about:
>>
>> Cache c = cacheManager.getCache( cacheName, classLoader );
>>
>> or
>> Cache c = cacheManager.getCache( cacheName 
>> ).usingClassLoader(classLoader );
>>
>> BTW if that's an issue on the API, maybe you should propose it to
>> JSR-107 as well ?
>
> We have a configurable Marshaller, right?  Then why don't we just use
> the class loader that the current Marshaller uses?

 +1
 I like the clean approach, not sure how you configure the "current
 Marshaller" to use the correct CL ?
 Likely hard to do via configuration file :)
>>>
>>> Well, the marshaller is a global component and so it's a cache manager 
>>> level. You can't make any assumptions about it's classloader, particularly 
>>> when lazy deserialization is configured and you want to make sure that the 
>>> data of the cache is deserialized with the correct classloader when the 
>>> user reads the data from the cache. This is gonna become even more 
>>> important when we for example move to having a single cache for all 2LC 
>>> entities or all EJB3 SFSBs where we'll definitely need multiple 
>>> classloaders to access a single cache.
>>>
>>
>> The current unmarshaller uses the TCCL, which is a great idea for
>> non-modular environments and will still work in AS7 for application
>> classes (so it's still a good default). It probably won't work if
>> Hibernate wants to store its own classes in the cache, because
>> Hibernate's internal classes may not be reachable from the
>> application's classloader.
>>
>> It gets even trickier if Hibernate wants to store a
>> PrivateHibernateCollection class in the cache containing instances of
>> application classes inside. Then I don't think there will be any
>> single classloader that could reach both the Hibernate classes and the
>> application classes so it can properly unmarshal both. Perhaps that's
>> just something for the Hibernate folks to worry about? Or maybe we
>> should allow the users to register more than one classloader with a
>> cache?
>
> You need to use a bridge classloader in this case.

You're right of course, Hibernate must have received/guessed the
application's classloader and so it is able to create a bridge
classloader that "includes" both.

And if the application classes include references to classes from
another module(s), the application has to provide a bridge classloader
to Hibernate anyway.

___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Pete Muir]

On 4 May 2011, at 05:34, Dan Berindei wrote:

> On Wed, May 4, 2011 at 10:14 AM, Galder Zamarreño  wrote:
>> 
>> On May 3, 2011, at 2:33 PM, Sanne Grinovero wrote:
>> 
>>> 2011/5/3 "이희승 (Trustin Lee)" :
 On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
> 2011/5/2 Manik Surtani :
>> 
>> On 1 May 2011, at 13:38, Pete Muir wrote:
>> 
> As in, user API?  That's a little intrusive... e.g., put(K, V, cl) ?
 
 Not for put, since you have the class, just get, and I was thinking
 something more like:
 
 Foo foo = getUsing(key, Foo.class)
>>> 
>>> This would be a pretty useful addition to the API anyway to avoid user 
>>> casts.
>> 
>> Maybe as an "advanced" API, so as not to pollute the basic API?  A bit 
>> like:
>> 
>> Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);
> 
> doesn't look much better than a cast, but is more cryptical :)
> 
> getting back to the classloader issue, what about:
> 
> Cache c = cacheManager.getCache( cacheName, classLoader );
> 
> or
> Cache c = cacheManager.getCache( cacheName ).usingClassLoader(classLoader 
> );
> 
> BTW if that's an issue on the API, maybe you should propose it to
> JSR-107 as well ?
 
 We have a configurable Marshaller, right?  Then why don't we just use
 the class loader that the current Marshaller uses?
>>> 
>>> +1
>>> I like the clean approach, not sure how you configure the "current
>>> Marshaller" to use the correct CL ?
>>> Likely hard to do via configuration file :)
>> 
>> Well, the marshaller is a global component and so it's a cache manager 
>> level. You can't make any assumptions about it's classloader, particularly 
>> when lazy deserialization is configured and you want to make sure that the 
>> data of the cache is deserialized with the correct classloader when the user 
>> reads the data from the cache. This is gonna become even more important when 
>> we for example move to having a single cache for all 2LC entities or all 
>> EJB3 SFSBs where we'll definitely need multiple classloaders to access a 
>> single cache.
>> 
> 
> The current unmarshaller uses the TCCL, which is a great idea for
> non-modular environments and will still work in AS7 for application
> classes (so it's still a good default). It probably won't work if
> Hibernate wants to store its own classes in the cache, because
> Hibernate's internal classes may not be reachable from the
> application's classloader.
> 
> It gets even trickier if Hibernate wants to store a
> PrivateHibernateCollection class in the cache containing instances of
> application classes inside. Then I don't think there will be any
> single classloader that could reach both the Hibernate classes and the
> application classes so it can properly unmarshal both. Perhaps that's
> just something for the Hibernate folks to worry about? Or maybe we
> should allow the users to register more than one classloader with a
> cache?

You need to use a bridge classloader in this case.
___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Pete Muir]

On 4 May 2011, at 09:55, Dan Berindei wrote:

> On Wed, May 4, 2011 at 7:18 AM, "이희승 (Trustin Lee)"  wrote:
>> On 05/03/2011 09:33 PM, Sanne Grinovero wrote:
>>> 2011/5/3 "이희승 (Trustin Lee)" :
 On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
> 2011/5/2 Manik Surtani :
>> 
>> On 1 May 2011, at 13:38, Pete Muir wrote:
>> 
> As in, user API?  That's a little intrusive... e.g., put(K, V, cl) ?
 
 Not for put, since you have the class, just get, and I was thinking
 something more like:
 
 Foo foo = getUsing(key, Foo.class)
>>> 
>>> This would be a pretty useful addition to the API anyway to avoid user 
>>> casts.
>> 
>> Maybe as an "advanced" API, so as not to pollute the basic API?  A bit 
>> like:
>> 
>> Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);
> 
> doesn't look much better than a cast, but is more cryptical :)
> 
> getting back to the classloader issue, what about:
> 
> Cache c = cacheManager.getCache( cacheName, classLoader );
> 
> or
> Cache c = cacheManager.getCache( cacheName ).usingClassLoader(classLoader 
> );
> 
> BTW if that's an issue on the API, maybe you should propose it to
> JSR-107 as well ?
 
 We have a configurable Marshaller, right?  Then why don't we just use
 the class loader that the current Marshaller uses?
>>> 
>>> +1
>>> I like the clean approach, not sure how you configure the "current
>>> Marshaller" to use the correct CL ?
>>> Likely hard to do via configuration file :)
>> 
>> By default, we could use the class loader that the current Unmarshaller
>> uses, and let user choose a class loader for a certain get() call.
>> 
>> So, we need to deal with the two issues here:
>> 
>> 1) Make sure that user can configure the default class loader in runtime
>> and calling get() (and consequently related unmarshaller) will use the
>> default class loader:
>> 
>>  cache.setDefaultClassLoader(UserClass.class.getClassLoader())
>> 
>> 2) Provide an additional API that allows a user to specify a class
>> loader for the current transaction or context:
>> 
>>  cache.get(K key, Class clazz) // +1 to Pete's suggestion
>> 
> 
> If V is Set then Set.class.getClassLoader() will
> give you the system classloader, which in most cases won't be able the
> MyObject class. It may not even be a Set of course, it could
> be just Set.
> 
> We could have a "shortcut" so the users can pass in a class instead of
> a classloader to avoid writing ".getClassLoader()", but we shouldn't
> require any relation between the class passed in and the class of the
> return value.

There are two forces at play here. Firstly the desire to introduce greater type 
safety into the Cache API, by returning a type for user, rather than just an 
object, and secondly the need to specify the CL.

We could have an API like:

 V get(Object key, Class classLoaderType);

and an overloaded form

 V get(Object key, ClassLoader cl);

This does involve Infinispan having to do a runtime cast to V without the class 
type passed in as a parameter, but this may be a necessary evil in the interest 
of improving the user api.

An alternative which is somewhat more verbose, somewhat safer, and somewhat 
more complex to explain (but ultimately more logical):

// Use the classloader of the expectedType, this would be the normal case
 V get(Object key, Class expectedType);

// Specify the classloader to use as a class
 V get(Object key, Class expectedType, Class classLoader);

// Specify the classloader to use as a classloader
 V get(Object key, Class expectedType, ClassLoader classLoader);

We could also include

// Use the TCCL
V get(Object key);

Any thoughts?


___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Dan Berindei]

On Wed, May 4, 2011 at 7:18 AM, "이희승 (Trustin Lee)"  wrote:
> On 05/03/2011 09:33 PM, Sanne Grinovero wrote:
>> 2011/5/3 "이희승 (Trustin Lee)" :
>>> On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
 2011/5/2 Manik Surtani :
>
> On 1 May 2011, at 13:38, Pete Muir wrote:
>
 As in, user API?  That's a little intrusive... e.g., put(K, V, cl) ?
>>>
>>> Not for put, since you have the class, just get, and I was thinking
>>> something more like:
>>>
>>> Foo foo = getUsing(key, Foo.class)
>>
>> This would be a pretty useful addition to the API anyway to avoid user 
>> casts.
>
> Maybe as an "advanced" API, so as not to pollute the basic API?  A bit 
> like:
>
> Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);

 doesn't look much better than a cast, but is more cryptical :)

 getting back to the classloader issue, what about:

 Cache c = cacheManager.getCache( cacheName, classLoader );

 or
 Cache c = cacheManager.getCache( cacheName ).usingClassLoader(classLoader 
 );

 BTW if that's an issue on the API, maybe you should propose it to
 JSR-107 as well ?
>>>
>>> We have a configurable Marshaller, right?  Then why don't we just use
>>> the class loader that the current Marshaller uses?
>>
>> +1
>> I like the clean approach, not sure how you configure the "current
>> Marshaller" to use the correct CL ?
>> Likely hard to do via configuration file :)
>
> By default, we could use the class loader that the current Unmarshaller
> uses, and let user choose a class loader for a certain get() call.
>
> So, we need to deal with the two issues here:
>
> 1) Make sure that user can configure the default class loader in runtime
> and calling get() (and consequently related unmarshaller) will use the
> default class loader:
>
>   cache.setDefaultClassLoader(UserClass.class.getClassLoader())
>
> 2) Provide an additional API that allows a user to specify a class
> loader for the current transaction or context:
>
>   cache.get(K key, Class clazz) // +1 to Pete's suggestion
>

If V is Set then Set.class.getClassLoader() will
give you the system classloader, which in most cases won't be able the
MyObject class. It may not even be a Set of course, it could
be just Set.

We could have a "shortcut" so the users can pass in a class instead of
a classloader to avoid writing ".getClassLoader()", but we shouldn't
require any relation between the class passed in and the class of the
return value.

Dan

___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[David M. Lloyd]

On 05/03/2011 11:18 PM, "이희승 (Trustin Lee)" wrote:
> By default, we could use the class loader that the current Unmarshaller
> uses, and let user choose a class loader for a certain get() call.
>
> So, we need to deal with the two issues here:
>
> 1) Make sure that user can configure the default class loader in runtime
> and calling get() (and consequently related unmarshaller) will use the
> default class loader:
>
> cache.setDefaultClassLoader(UserClass.class.getClassLoader())
>
> 2) Provide an additional API that allows a user to specify a class
> loader for the current transaction or context:
>
> cache.get(K key, Class  clazz) // +1 to Pete's suggestion

Agreed on all parts.  If the default CL isn't set then TCCL is probably 
a reasonable default.  But the user should be allowed to set the default 
class loader for resolution.  To be fair though I don't know whether 
that makes sense at a cache level, but from a marshalling and modularity 
perspective this makes a lot of sense.

-- 
- DML
___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Dan Berindei]

On Wed, May 4, 2011 at 10:14 AM, Galder Zamarreño  wrote:
>
> On May 3, 2011, at 2:33 PM, Sanne Grinovero wrote:
>
>> 2011/5/3 "이희승 (Trustin Lee)" :
>>> On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
 2011/5/2 Manik Surtani :
>
> On 1 May 2011, at 13:38, Pete Muir wrote:
>
 As in, user API?  That's a little intrusive... e.g., put(K, V, cl) ?
>>>
>>> Not for put, since you have the class, just get, and I was thinking
>>> something more like:
>>>
>>> Foo foo = getUsing(key, Foo.class)
>>
>> This would be a pretty useful addition to the API anyway to avoid user 
>> casts.
>
> Maybe as an "advanced" API, so as not to pollute the basic API?  A bit 
> like:
>
> Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);

 doesn't look much better than a cast, but is more cryptical :)

 getting back to the classloader issue, what about:

 Cache c = cacheManager.getCache( cacheName, classLoader );

 or
 Cache c = cacheManager.getCache( cacheName ).usingClassLoader(classLoader 
 );

 BTW if that's an issue on the API, maybe you should propose it to
 JSR-107 as well ?
>>>
>>> We have a configurable Marshaller, right?  Then why don't we just use
>>> the class loader that the current Marshaller uses?
>>
>> +1
>> I like the clean approach, not sure how you configure the "current
>> Marshaller" to use the correct CL ?
>> Likely hard to do via configuration file :)
>
> Well, the marshaller is a global component and so it's a cache manager level. 
> You can't make any assumptions about it's classloader, particularly when lazy 
> deserialization is configured and you want to make sure that the data of the 
> cache is deserialized with the correct classloader when the user reads the 
> data from the cache. This is gonna become even more important when we for 
> example move to having a single cache for all 2LC entities or all EJB3 SFSBs 
> where we'll definitely need multiple classloaders to access a single cache.
>

The current unmarshaller uses the TCCL, which is a great idea for
non-modular environments and will still work in AS7 for application
classes (so it's still a good default). It probably won't work if
Hibernate wants to store its own classes in the cache, because
Hibernate's internal classes may not be reachable from the
application's classloader.

It gets even trickier if Hibernate wants to store a
PrivateHibernateCollection class in the cache containing instances of
application classes inside. Then I don't think there will be any
single classloader that could reach both the Hibernate classes and the
application classes so it can properly unmarshal both. Perhaps that's
just something for the Hibernate folks to worry about? Or maybe we
should allow the users to register more than one classloader with a
cache?

Dan

___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Dan Berindei]

On Wed, May 4, 2011 at 10:16 AM, Galder Zamarreño  wrote:
>
> On May 3, 2011, at 8:06 AM, Dan Berindei wrote:
>>
>> I'm not even sure we should allow using more than one classloader,
>> otherwise a get operation might return an object loaded from the wrong
>> classloader. After all, we will only use the provided classloader if
>> we need to get the object from another node or if storeAsBinary is set
>> to true.
>
> This is not a right assumption. For example, we don't foresee supporting 
> asymmetric clusters in the short term as discussed in London, so this will 
> require all 2LC entities to be stored in the same cache, and these entities 
> could easily belong to a different classloaders.
>

I thought Hibernate has some other problems with storing entities in
the cache so it stores individual field values instead? If true, then
the cache would only store Hibernate classes.

Anyway, we already support the multiple classloaders scenario today by
using the TCCL, so I agree that it would be a bad idea to require a
single class loader per cache from now on.

Dan

___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Galder Zamarreño]

On May 3, 2011, at 8:06 AM, Dan Berindei wrote:

> I think Sanne's idea was that you shouldn't have to specify the class
> or class loader on every get, as it's very likely that all operations
> on that cache will use the same classloader. Most applications will
> only use one classloader anyway.
> 
> I'm not even sure we should allow using more than one classloader,
> otherwise a get operation might return an object loaded from the wrong
> classloader. After all, we will only use the provided classloader if
> we need to get the object from another node or if storeAsBinary is set
> to true.

This is not a right assumption. For example, we don't foresee supporting 
asymmetric clusters in the short term as discussed in London, so this will 
require all 2LC entities to be stored in the same cache, and these entities 
could easily belong to a different classloaders.

> 
> Dan
> 
> 
> On Mon, May 2, 2011 at 11:24 PM, Thomas P. Fuller
>  wrote:
>> How about just using "as" and "using" instead of "asClass" and
>> "usingClassloader"?
>> 
>> Consider something like the following which kind of looks like a dsl:
>> 
>> Foo f = cache.getAdvancedCache().as (Foo.class).using (classLoader).get
>> (key);
>> 
>> T
>> 
>> ____
>> From: Sanne Grinovero 
>> To: infinispan -Dev List 
>> Sent: Mon, 2 May, 2011 21:08:47
>> Subject: Re: [infinispan-dev] [Pull Request] Modular Classloading
>> Compatibility
>> 
>> 2011/5/2 Manik Surtani :
>>> 
>>> On 1 May 2011, at 13:38, Pete Muir wrote:
>>> 
>>>>>> As in, user API?  That's a little intrusive... e.g., put(K, V, cl) ?
>>>>> 
>>>>> Not for put, since you have the class, just get, and I was thinking
>>>>> something more like:
>>>>> 
>>>>> Foo foo = getUsing(key, Foo.class)
>>>> 
>>>> This would be a pretty useful addition to the API anyway to avoid user
>>>> casts.
>>> 
>>> Maybe as an "advanced" API, so as not to pollute the basic API?  A bit
>>> like:
>>> 
>>> Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);
>> 
>> doesn't look much better than a cast, but is more cryptical :)
>> 
>> getting back to the classloader issue, what about:
>> 
>> Cache c = cacheManager.getCache( cacheName, classLoader );
>> 
>> or
>> Cache c = cacheManager.getCache( cacheName ).usingClassLoader(classLoader );
>> 
>> BTW if that's an issue on the API, maybe you should propose it to
>> JSR-107 as well ?
>> 
>> Sanne
>> 
>> ___
>> infinispan-dev mailing list
>> infinispan-dev@lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>> ___
>> infinispan-dev mailing list
>> infinispan-dev@lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>> 
> 
> ___
> infinispan-dev mailing list
> infinispan-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Galder Zamarreño]

On May 3, 2011, at 2:33 PM, Sanne Grinovero wrote:

> 2011/5/3 "이희승 (Trustin Lee)" :
>> On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
>>> 2011/5/2 Manik Surtani :
 
 On 1 May 2011, at 13:38, Pete Muir wrote:
 
>>> As in, user API?  That's a little intrusive... e.g., put(K, V, cl) ?
>> 
>> Not for put, since you have the class, just get, and I was thinking
>> something more like:
>> 
>> Foo foo = getUsing(key, Foo.class)
> 
> This would be a pretty useful addition to the API anyway to avoid user 
> casts.
 
 Maybe as an "advanced" API, so as not to pollute the basic API?  A bit 
 like:
 
 Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);
>>> 
>>> doesn't look much better than a cast, but is more cryptical :)
>>> 
>>> getting back to the classloader issue, what about:
>>> 
>>> Cache c = cacheManager.getCache( cacheName, classLoader );
>>> 
>>> or
>>> Cache c = cacheManager.getCache( cacheName ).usingClassLoader(classLoader );
>>> 
>>> BTW if that's an issue on the API, maybe you should propose it to
>>> JSR-107 as well ?
>> 
>> We have a configurable Marshaller, right?  Then why don't we just use
>> the class loader that the current Marshaller uses?
> 
> +1
> I like the clean approach, not sure how you configure the "current
> Marshaller" to use the correct CL ?
> Likely hard to do via configuration file :)

Well, the marshaller is a global component and so it's a cache manager level. 
You can't make any assumptions about it's classloader, particularly when lazy 
deserialization is configured and you want to make sure that the data of the 
cache is deserialized with the correct classloader when the user reads the data 
from the cache. This is gonna become even more important when we for example 
move to having a single cache for all 2LC entities or all EJB3 SFSBs where 
we'll definitely need multiple classloaders to access a single cache. 

--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[이희승 (Trustin Lee)]

On 05/03/2011 09:33 PM, Sanne Grinovero wrote:
> 2011/5/3 "이희승 (Trustin Lee)" :
>> On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
>>> 2011/5/2 Manik Surtani :

 On 1 May 2011, at 13:38, Pete Muir wrote:

>>> As in, user API?  That's a little intrusive... e.g., put(K, V, cl) ?
>>
>> Not for put, since you have the class, just get, and I was thinking
>> something more like:
>>
>> Foo foo = getUsing(key, Foo.class)
>
> This would be a pretty useful addition to the API anyway to avoid user 
> casts.

 Maybe as an "advanced" API, so as not to pollute the basic API?  A bit 
 like:

 Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);
>>>
>>> doesn't look much better than a cast, but is more cryptical :)
>>>
>>> getting back to the classloader issue, what about:
>>>
>>> Cache c = cacheManager.getCache( cacheName, classLoader );
>>>
>>> or
>>> Cache c = cacheManager.getCache( cacheName ).usingClassLoader(classLoader );
>>>
>>> BTW if that's an issue on the API, maybe you should propose it to
>>> JSR-107 as well ?
>>
>> We have a configurable Marshaller, right?  Then why don't we just use
>> the class loader that the current Marshaller uses?
> 
> +1
> I like the clean approach, not sure how you configure the "current
> Marshaller" to use the correct CL ?
> Likely hard to do via configuration file :)

By default, we could use the class loader that the current Unmarshaller
uses, and let user choose a class loader for a certain get() call.

So, we need to deal with the two issues here:

1) Make sure that user can configure the default class loader in runtime
and calling get() (and consequently related unmarshaller) will use the
default class loader:

   cache.setDefaultClassLoader(UserClass.class.getClassLoader())

2) Provide an additional API that allows a user to specify a class
loader for the current transaction or context:

   cache.get(K key, Class clazz) // +1 to Pete's suggestion

-- 
Trustin Lee, http://gleamynode.net/



signature.asc
Description: OpenPGP digital signature
___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[David M. Lloyd]

On 05/03/2011 12:41 PM, Pete Muir wrote:
>
> On 2 May 2011, at 10:10, Manik Surtani wrote:
>
>>
>> On 1 May 2011, at 13:38, Pete Muir wrote:
>>
> As in, user API?  That's a little intrusive... e.g., put(K, V, cl) ?

 Not for put, since you have the class, just get, and I was thinking
 something more like:

 Foo foo = getUsing(key, Foo.class)
>>>
>>> This would be a pretty useful addition to the API anyway to avoid user 
>>> casts.
>>
>> Maybe as an "advanced" API, so as not to pollute the basic API?  A bit like:
>>
>> Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);
>>
>
> I don't get why you need the asClass() (or any other method call in here), 
> just declare a method
>
>   T get(key, Value.class);
>
> surely?

Yeah I agree, an asClass() method means an intermediate object and extra 
step which is a little clunky.  Adding one method doesn't hurt.

I'd like to see what the unmarshalling logic for this will look like - 
it is a novel and interesting, yet simple, solution; my favorite kind. :)
-- 
- DML
___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Pete Muir]

On 2 May 2011, at 10:10, Manik Surtani wrote:

> 
> On 1 May 2011, at 13:38, Pete Muir wrote:
> 
 As in, user API?  That's a little intrusive... e.g., put(K, V, cl) ?
>>> 
>>> Not for put, since you have the class, just get, and I was thinking 
>>> something more like:
>>> 
>>> Foo foo = getUsing(key, Foo.class)
>> 
>> This would be a pretty useful addition to the API anyway to avoid user casts.
> 
> Maybe as an "advanced" API, so as not to pollute the basic API?  A bit like:
> 
> Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);
> 

I don't get why you need the asClass() (or any other method call in here), just 
declare a method

 T get(key, Value.class);

surely?


___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Sanne Grinovero]

2011/5/3 "이희승 (Trustin Lee)" :
> On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
>> 2011/5/2 Manik Surtani :
>>>
>>> On 1 May 2011, at 13:38, Pete Muir wrote:
>>>
>> As in, user API?  That's a little intrusive... e.g., put(K, V, cl) ?
>
> Not for put, since you have the class, just get, and I was thinking
> something more like:
>
> Foo foo = getUsing(key, Foo.class)

 This would be a pretty useful addition to the API anyway to avoid user 
 casts.
>>>
>>> Maybe as an "advanced" API, so as not to pollute the basic API?  A bit like:
>>>
>>> Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);
>>
>> doesn't look much better than a cast, but is more cryptical :)
>>
>> getting back to the classloader issue, what about:
>>
>> Cache c = cacheManager.getCache( cacheName, classLoader );
>>
>> or
>> Cache c = cacheManager.getCache( cacheName ).usingClassLoader(classLoader );
>>
>> BTW if that's an issue on the API, maybe you should propose it to
>> JSR-107 as well ?
>
> We have a configurable Marshaller, right?  Then why don't we just use
> the class loader that the current Marshaller uses?

+1
I like the clean approach, not sure how you configure the "current
Marshaller" to use the correct CL ?
Likely hard to do via configuration file :)

Sanne

___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[이희승 (Trustin Lee)]

On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
> 2011/5/2 Manik Surtani :
>>
>> On 1 May 2011, at 13:38, Pete Muir wrote:
>>
> As in, user API?  That's a little intrusive... e.g., put(K, V, cl) ?

 Not for put, since you have the class, just get, and I was thinking
 something more like:

 Foo foo = getUsing(key, Foo.class)
>>>
>>> This would be a pretty useful addition to the API anyway to avoid user 
>>> casts.
>>
>> Maybe as an "advanced" API, so as not to pollute the basic API?  A bit like:
>>
>> Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);
> 
> doesn't look much better than a cast, but is more cryptical :)
> 
> getting back to the classloader issue, what about:
> 
> Cache c = cacheManager.getCache( cacheName, classLoader );
> 
> or
> Cache c = cacheManager.getCache( cacheName ).usingClassLoader(classLoader );
> 
> BTW if that's an issue on the API, maybe you should propose it to
> JSR-107 as well ?

We have a configurable Marshaller, right?  Then why don't we just use
the class loader that the current Marshaller uses?

-- 
Trustin Lee, http://gleamynode.net/



signature.asc
Description: OpenPGP digital signature
___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Dan Berindei]

I think Sanne's idea was that you shouldn't have to specify the class
or class loader on every get, as it's very likely that all operations
on that cache will use the same classloader. Most applications will
only use one classloader anyway.

I'm not even sure we should allow using more than one classloader,
otherwise a get operation might return an object loaded from the wrong
classloader. After all, we will only use the provided classloader if
we need to get the object from another node or if storeAsBinary is set
to true.

Dan


On Mon, May 2, 2011 at 11:24 PM, Thomas P. Fuller
 wrote:
> How about just using "as" and "using" instead of "asClass" and
> "usingClassloader"?
>
> Consider something like the following which kind of looks like a dsl:
>
> Foo f = cache.getAdvancedCache().as (Foo.class).using (classLoader).get
> (key);
>
> T
>
> 
> From: Sanne Grinovero 
> To: infinispan -Dev List 
> Sent: Mon, 2 May, 2011 21:08:47
> Subject: Re: [infinispan-dev] [Pull Request] Modular Classloading
> Compatibility
>
> 2011/5/2 Manik Surtani :
>>
>> On 1 May 2011, at 13:38, Pete Muir wrote:
>>
>>>>> As in, user API?  That's a little intrusive... e.g., put(K, V, cl) ?
>>>>
>>>> Not for put, since you have the class, just get, and I was thinking
>>>> something more like:
>>>>
>>>> Foo foo = getUsing(key, Foo.class)
>>>
>>> This would be a pretty useful addition to the API anyway to avoid user
>>> casts.
>>
>> Maybe as an "advanced" API, so as not to pollute the basic API?  A bit
>> like:
>>
>> Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);
>
> doesn't look much better than a cast, but is more cryptical :)
>
> getting back to the classloader issue, what about:
>
> Cache c = cacheManager.getCache( cacheName, classLoader );
>
> or
> Cache c = cacheManager.getCache( cacheName ).usingClassLoader(classLoader );
>
> BTW if that's an issue on the API, maybe you should propose it to
> JSR-107 as well ?
>
> Sanne
>
> ___
> infinispan-dev mailing list
> infinispan-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
> ___
> infinispan-dev mailing list
> infinispan-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>

___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Thomas P. Fuller]

How about just using "as" and "using" instead of "asClass" and 
"usingClassloader"?

Consider something like the following which kind of looks like a dsl:

Foo f = cache.getAdvancedCache().as (Foo.class).using (classLoader).get (key);

T





From: Sanne Grinovero 
To: infinispan -Dev List 
Sent: Mon, 2 May, 2011 21:08:47
Subject: Re: [infinispan-dev] [Pull Request] Modular Classloading Compatibility

2011/5/2 Manik Surtani :
>
> On 1 May 2011, at 13:38, Pete Muir wrote:
>
>>>> As in, user API?  That's a little intrusive... e.g., put(K, V, cl) ?
>>>
>>> Not for put, since you have the class, just get, and I was thinking
>>> something more like:
>>>
>>> Foo foo = getUsing(key, Foo.class)
>>
>> This would be a pretty useful addition to the API anyway to avoid user casts.
>
> Maybe as an "advanced" API, so as not to pollute the basic API?  A bit like:
>
> Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);

doesn't look much better than a cast, but is more cryptical :)

getting back to the classloader issue, what about:

Cache c = cacheManager.getCache( cacheName, classLoader );

or
Cache c = cacheManager.getCache( cacheName ).usingClassLoader(classLoader );

BTW if that's an issue on the API, maybe you should propose it to
JSR-107 as well ?

Sanne

___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Sanne Grinovero]

2011/5/2 Manik Surtani :
>
> On 1 May 2011, at 13:38, Pete Muir wrote:
>
 As in, user API?  That's a little intrusive... e.g., put(K, V, cl) ?
>>>
>>> Not for put, since you have the class, just get, and I was thinking
>>> something more like:
>>>
>>> Foo foo = getUsing(key, Foo.class)
>>
>> This would be a pretty useful addition to the API anyway to avoid user casts.
>
> Maybe as an "advanced" API, so as not to pollute the basic API?  A bit like:
>
> Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);

doesn't look much better than a cast, but is more cryptical :)

getting back to the classloader issue, what about:

Cache c = cacheManager.getCache( cacheName, classLoader );

or
Cache c = cacheManager.getCache( cacheName ).usingClassLoader(classLoader );

BTW if that's an issue on the API, maybe you should propose it to
JSR-107 as well ?

Sanne

___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Thomas P. Fuller]

I like this solution.

T





From: Manik Surtani 
To: infinispan -Dev List 
Sent: Mon, 2 May, 2011 15:10:03
Subject: Re: [infinispan-dev] [Pull Request] Modular Classloading Compatibility


On 1 May 2011, at 13:38, Pete Muir wrote:

>>> As in, user API?  That's a little intrusive... e.g., put(K, V, cl) ?
>> 
>> Not for put, since you have the class, just get, and I was thinking 
>> something more like:
>> 
>> Foo foo = getUsing(key, Foo.class)
> 
> This would be a pretty useful addition to the API anyway to avoid user casts.

Maybe as an "advanced" API, so as not to pollute the basic API?  A bit like:

Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);

--
Manik Surtani
ma...@jboss.org
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org




___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Manik Surtani]

On 1 May 2011, at 13:38, Pete Muir wrote:

>>> As in, user API?  That's a little intrusive... e.g., put(K, V, cl) ?
>> 
>> Not for put, since you have the class, just get, and I was thinking 
>> something more like:
>> 
>> Foo foo = getUsing(key, Foo.class)
> 
> This would be a pretty useful addition to the API anyway to avoid user casts.

Maybe as an "advanced" API, so as not to pollute the basic API?  A bit like:

Foo f = cache.getAdvancedCache().asClass(Foo.class).get(key);

--
Manik Surtani
ma...@jboss.org
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org




___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Pete Muir]

On 29 Apr 2011, at 12:45, Jason T. Greene wrote:

> On 4/28/11 4:52 PM, Manik Surtani wrote:
>> 
>> On 27 Apr 2011, at 12:39, Jason T. Greene wrote:
>> 
>>> Available here:
>>> https://github.com/infinispan/infinispan/pull/278
>>> 
>>> The problem is basically that infinispan currently is using TCCL for all
>>> class loading and resource loading. This has a lot of problems in
>>> modular containers (OSGi, AS7, etc), where you dont have framework
>>> implementation classes on the same classloader as user classes (that is
>>> how they achieve true isolation)
>>> 
>>> You can read about this in more detail here:
>>> http://community.jboss.org/wiki/ModuleCompatibleClassloadingGuide
>>> 
>>> The patch in the pull request is a first step, and should fix many
>>> issues, but it does not address all (there is still a lot of TCCL usage
>>> spread out among cacheloaders and so on), and ultimately it's just a
>>> work around. It should, however, be compatible in any other non-modular
>>> environment.
>>> 
>>> Really the ultimate solution is to setup a proper demarcation between
>>> what the user is supposed to provide, and what is expected to be bundled
>>> with infinispan. Whenever there is something the user can provide a
>>> class, then the API should accept a classloader to load that class from.
>> 
>> As in, user API?  That's a little intrusive... e.g., put(K, V, cl) ?
> 
> Not for put, since you have the class, just get, and I was thinking 
> something more like:
> 
> Foo foo = getUsing(key, Foo.class)

This would be a pretty useful addition to the API anyway to avoid user casts.

> 
> This could verify type compatibility and use the classloader of the 
> type. But that does of course mean more methods on AdvancedCache (not so 
> great).
> 
> Alternatively you could just have a method on the invocation context. 
> For friendlier interaction and elimination of thread locals, you could 
> introduce a session based API.
> 
> CacheSession session = CacheSession.from(cache);
> session.setClassLoader(blah);
> session.setOtherInterestingOptionsLikeBatchingEtc
> 
> session.get/put etc
> 
> -- 
> Jason T. Greene
> JBoss, a division of Red Hat
> ___
> infinispan-dev mailing list
> infinispan-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Jason T. Greene]

On 4/28/11 4:52 PM, Manik Surtani wrote:
>
> On 27 Apr 2011, at 12:39, Jason T. Greene wrote:
>
>> Available here:
>> https://github.com/infinispan/infinispan/pull/278
>>
>> The problem is basically that infinispan currently is using TCCL for all
>> class loading and resource loading. This has a lot of problems in
>> modular containers (OSGi, AS7, etc), where you dont have framework
>> implementation classes on the same classloader as user classes (that is
>> how they achieve true isolation)
>>
>> You can read about this in more detail here:
>> http://community.jboss.org/wiki/ModuleCompatibleClassloadingGuide
>>
>> The patch in the pull request is a first step, and should fix many
>> issues, but it does not address all (there is still a lot of TCCL usage
>> spread out among cacheloaders and so on), and ultimately it's just a
>> work around. It should, however, be compatible in any other non-modular
>> environment.
>>
>> Really the ultimate solution is to setup a proper demarcation between
>> what the user is supposed to provide, and what is expected to be bundled
>> with infinispan. Whenever there is something the user can provide a
>> class, then the API should accept a classloader to load that class from.
>
> As in, user API?  That's a little intrusive... e.g., put(K, V, cl) ?

Not for put, since you have the class, just get, and I was thinking 
something more like:

Foo foo = getUsing(key, Foo.class)

This could verify type compatibility and use the classloader of the 
type. But that does of course mean more methods on AdvancedCache (not so 
great).

Alternatively you could just have a method on the invocation context. 
For friendlier interaction and elimination of thread locals, you could 
introduce a session based API.

CacheSession session = CacheSession.from(cache);
session.setClassLoader(blah);
session.setOtherInterestingOptionsLikeBatchingEtc

session.get/put etc

-- 
Jason T. Greene
JBoss, a division of Red Hat
___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

[Manik Surtani]

On 27 Apr 2011, at 12:39, Jason T. Greene wrote:

> Available here:
> https://github.com/infinispan/infinispan/pull/278
> 
> The problem is basically that infinispan currently is using TCCL for all 
> class loading and resource loading. This has a lot of problems in 
> modular containers (OSGi, AS7, etc), where you dont have framework 
> implementation classes on the same classloader as user classes (that is 
> how they achieve true isolation)
> 
> You can read about this in more detail here:
> http://community.jboss.org/wiki/ModuleCompatibleClassloadingGuide
> 
> The patch in the pull request is a first step, and should fix many 
> issues, but it does not address all (there is still a lot of TCCL usage 
> spread out among cacheloaders and so on), and ultimately it's just a 
> work around. It should, however, be compatible in any other non-modular 
> environment.
> 
> Really the ultimate solution is to setup a proper demarcation between 
> what the user is supposed to provide, and what is expected to be bundled 
> with infinispan. Whenever there is something the user can provide a 
> class, then the API should accept a classloader to load that class from. 

As in, user API?  That's a little intrusive... e.g., put(K, V, cl) ?

> If it's a class that is just internal wiring of infinispan, then 
> Infinispan's defining classloader should always be used.
> 
> The one case I can think of in infnispan where TCCL really makes sense 
> is in the case of lazy deserialization from an EE application. However 
> that is only guaranteed to work when you are executing in a context that 
> has that style of contract (like in an EE component). There are many 
> other cases though where someone would expect it to work from a non-EE 
> context (e.g. from a thread pool). What you really want is the caller's 
> classloader, but that is not cheap to get at, so it's something that 
> should be supplied as part of the API interaction (in the case where 
> there is no EE context). Alternatively you can just just require that 
> folks push/pop TCCL, but users often get this wrong.
> 
> Thanks!
> 
> -- 
> Jason T. Greene
> JBoss, a division of Red Hat
> ___
> infinispan-dev mailing list
> infinispan-dev@lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Manik Surtani
ma...@jboss.org
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org



___
infinispan-dev mailing list
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev