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

2011-06-08 Thread 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

2011-06-08 Thread 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

2011-06-08 Thread 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

2011-06-08 Thread 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

2011-05-20 Thread 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

2011-05-20 Thread 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

2011-05-19 Thread Dan Berindei
On Wed, May 18, 2011 at 7:06 PM, Manik Surtani ma...@jboss.org 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 supposed to provide, and what is expected to be bundled
 with infinispan. Whenever there 

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

2011-05-19 Thread 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

2011-05-19 Thread 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

2011-05-19 Thread 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, 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

2011-05-19 Thread Dan Berindei
On Thu, May 19, 2011 at 1:31 PM, Galder Zamarreño gal...@redhat.com 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

2011-05-18 Thread Manik Surtani

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

 
 If V is SetMyObject then SetMyObject.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 SetMyObject 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

2011-05-16 Thread 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 da...@redhat.com wrote:
 On 11/05/2011 17:54, Dan Berindei wrote:
 On Wed, May 11, 2011 at 7:08 PM, Pete Muirpm...@redhat.com  wrote:
 Were we developing for OSGi I would certainly agree with you. However in 
 many environments today we can reasonably expect the TCCL to be set and to 
 be able to load the classes we need. So whilst making it part of the API 
 is the safest option, it's also making complicated an API for the sake of 
 the few at the cost of the many. Further this also seems kinda nasty to 
 me. We know the class (and hence bundle/module) when we put the object 
 into Infinispan, therefore why do we require people to respecify this 
 again?
 
 David, can we not actually do something here akin to what we are 
 discussing for Weld? Whereby we can serialize out the bundle id and then 
 find the correct CL based on that when we deserialize.
 What if the object is a java.util.ArrayList? Each element in the list
 could belong to a different bundle, so you'd have to write a bundle id
 for every element in the list.
 Yes, if you know the Bundle-SymbolicName and Version (or the Bundle ID)
 you can find its classloader.
 
 On the other question, if you're passing in a class object then you can
 obtain its classloader and hence the bundle where it came from. But, and
 I think this is what Dan allused to above, is it always true that the
 class your passing in comes from the bundle that you need to have or
 could it also come from one of its parent class loaders?
 
 
 Exactly David, sorry if my message was a little cryptic. I think in
 order to handle every case properly you would have to go through the
 entire object graph being stored in the cache in order to find all the
 classloaders/bundle ids that you will need on get().
 
 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

2011-05-16 Thread 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 da...@redhat.com wrote:
 On 11/05/2011 17:54, Dan Berindei wrote:
 On Wed, May 11, 2011 at 7:08 PM, Pete Muirpm...@redhat.com  wrote:
 Were we developing for OSGi I would certainly agree with you. However in 
 many environments today we can reasonably expect the TCCL to be set and 
 to be able to load the classes we need. So whilst making it part of the 
 API is the safest option, it's also making complicated an API for the 
 sake of the few at the cost of the many. Further this also seems kinda 
 nasty to me. We know the class (and hence bundle/module) when we put the 
 object into Infinispan, therefore why do we require people to respecify 
 this again?
 
 David, can we not actually do something here akin to what we are 
 discussing for Weld? Whereby we can serialize out the bundle id and then 
 find the correct CL based on that when we deserialize.
 What if the object is a java.util.ArrayList? Each element in the list
 could belong to a different bundle, so you'd have to write a bundle id
 for every element in the list.
 Yes, if you know the Bundle-SymbolicName and Version (or the Bundle ID)
 you can find its classloader.
 
 On the other question, if you're passing in a class object then you can
 obtain its classloader and hence the bundle where it came from. But, and
 I think this is what Dan allused to above, is it always true that the
 class your passing in comes from the bundle that you need to have or
 could it also come from one of its parent class loaders?
 
 
 Exactly David, sorry if my message was a little cryptic. I think in
 order to handle every case properly you would have to go through the
 entire object graph being stored in the cache in order to find all the
 classloaders/bundle ids that you will need on get().
 
 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

2011-05-16 Thread 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, ClassV 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 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 da...@redhat.com wrote:
 On 11/05/2011 17:54, Dan Berindei wrote:
 On Wed, May 11, 2011 at 7:08 PM, Pete Muirpm...@redhat.com  wrote:
 Were we developing for OSGi I would certainly agree with you. However in 
 many environments today we can reasonably expect the TCCL to be set and 
 to be able to load the classes we need. So whilst making it part of the 
 API is the safest option, it's also making complicated an API for the 
 sake of the few at the cost of the many. Further this also seems kinda 
 nasty to me. We know the class (and hence bundle/module) when we put the 
 object into Infinispan, therefore why do we require people to respecify 
 this again?

 David, can we not actually do something here akin to what we are 
 discussing for Weld? Whereby we can serialize out the bundle id and then 
 find the correct CL based on that when we deserialize.
 What if the object is a java.util.ArrayList? Each element in the list
 could belong to a different bundle, so you'd have to write a bundle id
 for every element in the list.
 Yes, if you know the Bundle-SymbolicName and Version (or the Bundle ID)
 you can find its classloader.

 On the other question, if you're passing in a class object then you can
 obtain its classloader and hence the bundle where it came from. But, and
 I think this is what Dan allused to above, is it always true that the
 class your passing in comes from the bundle that you need to have or
 could it also come from one of its parent class loaders?


 Exactly David, sorry if my message was a little cryptic. I think in
 order to handle every case properly you would have to go through the
 entire object graph being stored in the cache in order to find all the
 classloaders/bundle ids that you will need on get().

 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


___
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

2011-05-16 Thread 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, ClassV  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 Muirpm...@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 Bosschaertda...@redhat.com  
 wrote:
 On 11/05/2011 17:54, Dan Berindei wrote:
 On Wed, May 11, 2011 at 7:08 PM, Pete Muirpm...@redhat.comwrote:
 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 

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

2011-05-16 Thread 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 gal...@redhat.comwrote:


 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, ClassV 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 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 da...@redhat.com
 wrote:
  On 11/05/2011 17:54, Dan Berindei wrote:
  On Wed, May 11, 2011 at 7:08 PM, Pete Muirpm...@redhat.com
  wrote:
  Were we developing for OSGi I would certainly agree with you.
 However in many environments today we can reasonably expect the TCCL to be
 set and to be able to load the classes we need. So whilst making it part of
 the API is the safest option, it's also making complicated an API for the
 sake of the few at the cost of the many. Further this also seems kinda nasty
 to me. We know the class (and hence bundle/module) when we put the object
 into Infinispan, therefore why do we require people to respecify this again?
 
  David, can we not actually do something here akin to what we are
 discussing for Weld? Whereby we can serialize out the bundle id and then
 find the correct CL based on that when we deserialize.
  What if the object is a java.util.ArrayList? Each element in the
 list
  could belong to a different bundle, so you'd have to write a bundle
 id
  for every element in the list.
  Yes, if you know the Bundle-SymbolicName and Version (or the Bundle
 ID)
  you can find its classloader.
 
  On the other question, if you're passing in a class object then you
 can
  obtain its classloader and hence the bundle where it came from. But,
 and
  I think this is what Dan allused to above, is it always true that the
  class your passing in comes from the bundle that you need to have or
  could it also come from one of its parent class loaders?
 
 
  Exactly David, sorry if my message was a little cryptic. I think in
  order to handle every case properly you would have to go through the
  entire object graph being stored in the cache in order to find all the
  classloaders/bundle ids that you will need on get().
 
  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 

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

2011-05-16 Thread Sanne Grinovero
2011/5/16 Pete Muir pm...@redhat.com:
 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, ClassV 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 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 da...@redhat.com 
 wrote:
 On 11/05/2011 17:54, Dan Berindei wrote:
 On Wed, May 11, 2011 at 7:08 PM, Pete Muirpm...@redhat.com  wrote:
 Were we developing for OSGi I would certainly agree with you. However 
 in many environments today we can reasonably expect the TCCL to be set 
 and to be able to load the classes we need. So whilst making it part 
 of the API is the safest option, it's also making complicated an API 
 for the sake of the few at the cost of the many. Further this also 
 seems kinda nasty to me. We know the class (and hence bundle/module) 
 when we put the object into Infinispan, therefore why do we require 
 people to respecify this again?

 David, can we not actually do something here akin to what we are 
 discussing for Weld? Whereby we can serialize out the bundle id and 
 then find the correct CL based on that when we deserialize.
 What if the object is a java.util.ArrayList? Each element in the list
 could belong to a different bundle, so you'd have to write a bundle id
 for every element in the list.
 Yes, if you know the Bundle-SymbolicName and Version (or the Bundle ID)
 you can find its classloader.

 On the other question, if you're passing in a class object then you can
 obtain its classloader and hence the bundle where it came from. But, and
 I think this is what Dan allused to above, is it always true that the
 class your passing in comes from the bundle that you need to have or
 could it also come from one of its parent class loaders?


 Exactly David, sorry if my message was a little cryptic. I think in
 order to handle every case properly you would have to go through the
 entire object graph being stored in the cache in order to find all the
 classloaders/bundle ids that you will need on get().

 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 

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

2011-05-16 Thread 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 pm...@redhat.com:
 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

2011-05-16 Thread 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 david.ll...@redhat.com 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 gal...@redhat.com
 mailto:gal...@redhat.com 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, ClassV 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 pm...@redhat.com 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
     da...@redhat.com mailto:da...@redhat.com wrote:
       On 11/05/2011 17:54, Dan Berindei wrote:
       On Wed, May 11, 2011 at 7:08 PM, Pete Muirpm...@redhat.com
     mailto:pm...@redhat.com  wrote:
       Were we developing for OSGi I would certainly agree with
     you. However in many environments today we can reasonably expect the
     TCCL to be set and to be able to load the classes we need. So whilst
     making it part of the API is the safest option, it's also making
     complicated an API for the sake of the few at the cost of the many.
     Further this also seems kinda nasty to me. We know the class (and
     hence bundle/module) when we put the object into Infinispan,
     therefore why do we require people to respecify this again?
      
       David, can we not actually do something here akin to what
     we are discussing for Weld? Whereby we can serialize out the bundle
     id and then find the correct CL based on that when we deserialize.
       What if the object is a java.util.ArrayList? Each element in
     the list
       could belong to a different bundle, so you'd have to write a
     bundle id
       for every element in the list.
       Yes, if you know the Bundle-SymbolicName and Version (or the
     Bundle ID)
       you can find its classloader.
      
       On the other question, if you're passing in a class object
     then you can
       obtain its classloader and hence the bundle where it came
     from. But, and
       I think this is what Dan allused to above, is it always true
     that the
       class your passing in comes from the bundle that you need to
     have or
       could it also come from one of its parent class loaders?
      
      
       Exactly David, sorry if my message was a little cryptic. I
     think in
       order to handle every case properly you would have to go
     through the
       entire object graph being 

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

2011-05-13 Thread Jason T. Greene
On 5/12/11 4:18 AM, Dan Berindei wrote:
 On Wed, May 11, 2011 at 11:18 PM, David Bosschaertda...@redhat.com  wrote:
 On 11/05/2011 17:54, Dan Berindei wrote:
 On Wed, May 11, 2011 at 7:08 PM, Pete Muirpm...@redhat.comwrote:
 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

2011-05-13 Thread Dan Berindei
On Fri, May 13, 2011 at 5:48 PM, Jason T. Greene
jason.gre...@redhat.com wrote:
 On 5/12/11 4:18 AM, Dan Berindei wrote:

 On Wed, May 11, 2011 at 11:18 PM, David Bosschaertda...@redhat.com
  wrote:

 On 11/05/2011 17:54, Dan Berindei wrote:

 On Wed, May 11, 2011 at 7:08 PM, Pete Muirpm...@redhat.com    wrote:

 Were we developing for OSGi I would certainly agree with you. However
 in many environments today we can reasonably expect the TCCL to be set and
 to be able to load the classes we need. So whilst making it part of the 
 API
 is the safest option, it's also making complicated an API for the sake of
 the few at the cost of the many. Further this also seems kinda nasty to 
 me.
 We know the class (and hence bundle/module) when we put the object into
 Infinispan, therefore why do we require people to respecify this again?

 David, can we not actually do something here akin to what we are
 discussing for Weld? Whereby we can serialize out the bundle id and then
 find the correct CL based on that when we deserialize.

 What if the object is a java.util.ArrayList? Each element in the list
 could belong to a different bundle, so you'd have to write a bundle id
 for every element in the list.

 Yes, if you know the Bundle-SymbolicName and Version (or the Bundle ID)
 you can find its classloader.

 On the other question, if you're passing in a class object then you can
 obtain its classloader and hence the bundle where it came from. But, and
 I think this is what Dan allused to above, is it always true that the
 class your passing in comes from the bundle that you need to have or
 could it also come from one of its parent class loaders?


 Exactly David, sorry if my message was a little cryptic. I think in
 order to handle every case properly you would have to go through the
 entire object graph being stored in the cache in order to find all the
 classloaders/bundle ids that you will need on get().

 This approach just doesn't scale, and it would not work in all environments
 (there is no gaurantee you can get the original classloader if the
 environment doesn't allow you to look them up by some kind of unique id).

 That seems like a lot of overhead to me, and forcing the user to
 provide the classloader doesn't seem that bad in comparison. Perhaps
 we should use something other than a thread-local for this though

 Yes thread locals are brittle, and tend to leak if they aren't managed
 correctly. Using a context specific instance is a much better approach.

 so
 that users can do a onto the result of a
 cacheManager.getCache(A).usingClassLoader(A.class) and never have to
 provide the classloader again.


 That's similar to what I was proposing with the CacheSession notion. The
 basic notion is that you move thread/context specific state to a separate
 object that the caller uses (optionaly), and it mirrors the same Cache API.


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

 In fact I think this is a good idea for the invocation flags we
 already have, too. It would involve creating lots of overloads in
 CacheDelegate with a PreInvocationContext parameter and a new
 CacheDelegateWithContext class to invoke those methods, but the public
 API would remain the same.

 You dont need to reuse the same impl. Just create a new delegate which
 passes an extra invocation state parameter with ever call. The overhead of
 that is tiny.


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

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

Cheers
Dan

___
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

2011-05-12 Thread Dan Berindei
On Wed, May 11, 2011 at 11:18 PM, David Bosschaert da...@redhat.com wrote:
 On 11/05/2011 17:54, Dan Berindei wrote:
 On Wed, May 11, 2011 at 7:08 PM, Pete Muirpm...@redhat.com  wrote:
 Were we developing for OSGi I would certainly agree with you. However in 
 many environments today we can reasonably expect the TCCL to be set and to 
 be able to load the classes we need. So whilst making it part of the API is 
 the safest option, it's also making complicated an API for the sake of the 
 few at the cost of the many. Further this also seems kinda nasty to me. We 
 know the class (and hence bundle/module) when we put the object into 
 Infinispan, therefore why do we require people to respecify this again?

 David, can we not actually do something here akin to what we are discussing 
 for Weld? Whereby we can serialize out the bundle id and then find the 
 correct CL based on that when we deserialize.
 What if the object is a java.util.ArrayList? Each element in the list
 could belong to a different bundle, so you'd have to write a bundle id
 for every element in the list.
 Yes, if you know the Bundle-SymbolicName and Version (or the Bundle ID)
 you can find its classloader.

 On the other question, if you're passing in a class object then you can
 obtain its classloader and hence the bundle where it came from. But, and
 I think this is what Dan allused to above, is it always true that the
 class your passing in comes from the bundle that you need to have or
 could it also come from one of its parent class loaders?


Exactly David, sorry if my message was a little cryptic. I think in
order to handle every case properly you would have to go through the
entire object graph being stored in the cache in order to find all the
classloaders/bundle ids that you will need on get().

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

2011-05-12 Thread Dan Berindei
On Thu, May 12, 2011 at 1:22 PM, Emmanuel Bernard
emman...@hibernate.org 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

2011-05-11 Thread 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 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 

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

2011-05-11 Thread David Bosschaert
On 11/05/2011 17:54, Dan Berindei wrote:
 On Wed, May 11, 2011 at 7:08 PM, Pete Muirpm...@redhat.com  wrote:
 Were we developing for OSGi I would certainly agree with you. However in 
 many environments today we can reasonably expect the TCCL to be set and to 
 be able to load the classes we need. So whilst making it part of the API is 
 the safest option, it's also making complicated an API for the sake of the 
 few at the cost of the many. Further this also seems kinda nasty to me. We 
 know the class (and hence bundle/module) when we put the object into 
 Infinispan, therefore why do we require people to respecify this again?

 David, can we not actually do something here akin to what we are discussing 
 for Weld? Whereby we can serialize out the bundle id and then find the 
 correct CL based on that when we deserialize.
 What if the object is a java.util.ArrayList? Each element in the list
 could belong to a different bundle, so you'd have to write a bundle id
 for every element in the list.
Yes, if you know the Bundle-SymbolicName and Version (or the Bundle ID) 
you can find its classloader.

On the other question, if you're passing in a class object then you can 
obtain its classloader and hence the bundle where it came from. But, and 
I think this is what Dan allused to above, is it always true that the 
class your passing in comes from the bundle that you need to have or 
could it also come from one of its parent class loaders?

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

2011-05-10 Thread 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 Muirpm...@redhat.com  wrote:
 On 4 May 2011, at 05:34, Dan Berindei wrote:
 
 On Wed, May 4, 2011 at 10:14 AM, Galder Zamarreñogal...@redhat.com 
  wrote:
 On May 3, 2011, at 2:33 PM, Sanne Grinovero wrote:
 
 2011/5/3 이희승 (Trustin Lee)trus...@gmail.com:
 On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
 2011/5/2 Manik Surtanima...@jboss.org:
 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 = 

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

2011-05-09 Thread 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 Muirpm...@redhat.com  wrote:
 On 4 May 2011, at 05:34, Dan Berindei wrote:

 On Wed, May 4, 2011 at 10:14 AM, Galder Zamarreñogal...@redhat.com  
 wrote:
 On May 3, 2011, at 2:33 PM, Sanne Grinovero wrote:

 2011/5/3 이희승 (Trustin Lee)trus...@gmail.com:
 On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
 2011/5/2 Manik Surtanima...@jboss.org:
 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 

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

2011-05-09 Thread 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 Muirpm...@redhat.com  wrote:
 On 4 May 2011, at 05:34, Dan Berindei wrote:
 
 On Wed, May 4, 2011 at 10:14 AM, Galder Zamarreñogal...@redhat.com  
 wrote:
 On May 3, 2011, at 2:33 PM, Sanne Grinovero wrote:
 
 2011/5/3 이희승 (Trustin Lee)trus...@gmail.com:
 On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
 2011/5/2 Manik Surtanima...@jboss.org:
 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 

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

2011-05-09 Thread 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 Muirpm...@redhat.com  wrote:
 On 4 May 2011, at 05:34, Dan Berindei wrote:
 
 On Wed, May 4, 2011 at 10:14 AM, Galder Zamarreñogal...@redhat.com  
 wrote:
 On May 3, 2011, at 2:33 PM, Sanne Grinovero wrote:
 
 2011/5/3 이희승 (Trustin Lee)trus...@gmail.com:
 On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
 2011/5/2 Manik Surtanima...@jboss.org:
 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, 

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

2011-05-09 Thread 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 feedback.

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

 On Wed, May 4, 2011 at 5:09 PM, Pete Muirpm...@redhat.com   wrote:
 On 4 May 2011, at 05:34, Dan Berindei wrote:

 On Wed, May 4, 2011 at 10:14 AM, Galder Zamarreñogal...@redhat.com  
  wrote:
 On May 3, 2011, at 2:33 PM, Sanne Grinovero wrote:

 2011/5/3 이희승 (Trustin Lee)trus...@gmail.com:
 On 05/03/2011 05:08 AM, Sanne 

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

2011-05-07 Thread Dan Berindei
On Thu, May 5, 2011 at 10:05 PM, Emmanuel Bernard
emman...@hibernate.org 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 pm...@redhat.com wrote:

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

 On Wed, May 4, 2011 at 10:14 AM, Galder Zamarreño gal...@redhat.com 
 wrote:

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

 2011/5/3 이희승 (Trustin Lee) trus...@gmail.com:
 On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
 2011/5/2 Manik Surtani ma...@jboss.org:

 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 

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

2011-05-05 Thread 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) trus...@gmail.com 
 wrote:
 On 05/03/2011 09:33 PM, Sanne Grinovero wrote:
 2011/5/3 이희승 (Trustin Lee) trus...@gmail.com:
 On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
 2011/5/2 Manik Surtani ma...@jboss.org:
 
 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, ClassV clazz) // +1 to Pete's suggestion
 
 
 If V is SetMyObject then SetMyObject.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 SetMyObject 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, CL V get(Object key, ClassCL classLoaderType);
 
 and an overloaded form
 
 V 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 V get(Object key, ClassV expectedType);
 
 // Specify the classloader to use as a class
 V, CL V get(Object key, ClassV expectedType, ClassCL classLoader);
 
 // Specify the classloader to use as a classloader
 V V get(Object key, ClassV 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

2011-05-05 Thread 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 pm...@redhat.com wrote:
 
 On 4 May 2011, at 05:34, Dan Berindei wrote:
 
 On Wed, May 4, 2011 at 10:14 AM, Galder Zamarreño gal...@redhat.com wrote:
 
 On May 3, 2011, at 2:33 PM, Sanne Grinovero wrote:
 
 2011/5/3 이희승 (Trustin Lee) trus...@gmail.com:
 On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
 2011/5/2 Manik Surtani ma...@jboss.org:
 
 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

2011-05-05 Thread 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) trus...@gmail.com 
 wrote:
 On 05/03/2011 09:33 PM, Sanne Grinovero wrote:
 2011/5/3 이희승 (Trustin Lee) trus...@gmail.com:
 On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
 2011/5/2 Manik Surtani ma...@jboss.org:
 
 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, ClassV clazz) // +1 to Pete's suggestion
 
 
 If V is SetMyObject then SetMyObject.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 SetMyObject 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, CL V get(Object key, ClassCL classLoaderType);
 
 and an overloaded form
 
 V 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 V get(Object key, ClassV expectedType);
 
 // Specify the classloader to use as a class
 V, CL V get(Object key, ClassV expectedType, ClassCL classLoader);
 
 // Specify the classloader to use as a classloader
 V V get(Object key, ClassV 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
infinispan-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev

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

2011-05-05 Thread 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

2011-05-05 Thread 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

2011-05-05 Thread 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 pm...@redhat.com wrote:
 
 On 4 May 2011, at 05:34, Dan Berindei wrote:
 
 On Wed, May 4, 2011 at 10:14 AM, Galder Zamarreño gal...@redhat.com 
 wrote:
 
 On May 3, 2011, at 2:33 PM, Sanne Grinovero wrote:
 
 2011/5/3 이희승 (Trustin Lee) trus...@gmail.com:
 On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
 2011/5/2 Manik Surtani ma...@jboss.org:
 
 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 

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

2011-05-04 Thread Galder Zamarreño

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

 2011/5/3 이희승 (Trustin Lee) trus...@gmail.com:
 On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
 2011/5/2 Manik Surtani ma...@jboss.org:
 
 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

2011-05-04 Thread 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
 thomas.ful...@coherentlogic.com 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 sanne.grinov...@gmail.com
 To: infinispan -Dev List infinispan-dev@lists.jboss.org
 Sent: Mon, 2 May, 2011 21:08:47
 Subject: Re: [infinispan-dev] [Pull Request] Modular Classloading
 Compatibility
 
 2011/5/2 Manik Surtani ma...@jboss.org:
 
 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

2011-05-04 Thread Dan Berindei
On Wed, May 4, 2011 at 10:14 AM, Galder Zamarreño gal...@redhat.com wrote:

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

 2011/5/3 이희승 (Trustin Lee) trus...@gmail.com:
 On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
 2011/5/2 Manik Surtani ma...@jboss.org:

 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

2011-05-04 Thread Dan Berindei
On Wed, May 4, 2011 at 7:18 AM, 이희승 (Trustin Lee) trus...@gmail.com wrote:
 On 05/03/2011 09:33 PM, Sanne Grinovero wrote:
 2011/5/3 이희승 (Trustin Lee) trus...@gmail.com:
 On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
 2011/5/2 Manik Surtani ma...@jboss.org:

 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, ClassV clazz) // +1 to Pete's suggestion


If V is SetMyObject then SetMyObject.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 SetMyObject 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

2011-05-04 Thread Pete Muir

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

 On Wed, May 4, 2011 at 7:18 AM, 이희승 (Trustin Lee) trus...@gmail.com wrote:
 On 05/03/2011 09:33 PM, Sanne Grinovero wrote:
 2011/5/3 이희승 (Trustin Lee) trus...@gmail.com:
 On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
 2011/5/2 Manik Surtani ma...@jboss.org:
 
 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, ClassV clazz) // +1 to Pete's suggestion
 
 
 If V is SetMyObject then SetMyObject.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 SetMyObject 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, CL V get(Object key, ClassCL classLoaderType);

and an overloaded form

V 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 V get(Object key, ClassV expectedType);

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

// Specify the classloader to use as a classloader
V V get(Object key, ClassV 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

2011-05-04 Thread Pete Muir

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

 On Wed, May 4, 2011 at 10:14 AM, Galder Zamarreño gal...@redhat.com wrote:
 
 On May 3, 2011, at 2:33 PM, Sanne Grinovero wrote:
 
 2011/5/3 이희승 (Trustin Lee) trus...@gmail.com:
 On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
 2011/5/2 Manik Surtani ma...@jboss.org:
 
 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

2011-05-04 Thread Dan Berindei
On Wed, May 4, 2011 at 5:09 PM, Pete Muir pm...@redhat.com wrote:

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

 On Wed, May 4, 2011 at 10:14 AM, Galder Zamarreño gal...@redhat.com wrote:

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

 2011/5/3 이희승 (Trustin Lee) trus...@gmail.com:
 On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
 2011/5/2 Manik Surtani ma...@jboss.org:

 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

2011-05-04 Thread Dan Berindei
2011/5/4 Pete Muir pm...@redhat.com:

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

 On Wed, May 4, 2011 at 7:18 AM, 이희승 (Trustin Lee) trus...@gmail.com 
 wrote:
 On 05/03/2011 09:33 PM, Sanne Grinovero wrote:
 2011/5/3 이희승 (Trustin Lee) trus...@gmail.com:
 On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
 2011/5/2 Manik Surtani ma...@jboss.org:

 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, ClassV clazz) // +1 to Pete's suggestion


 If V is SetMyObject then SetMyObject.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 SetMyObject 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, CL V get(Object key, ClassCL classLoaderType);

 and an overloaded form

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


CacheK,V extends MapK,V, 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:

CL V get(Object key, ClassCL 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 V get(Object key, ClassV expectedType);

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

 // Specify the classloader to use as a classloader
 V V get(Object key, ClassV 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

2011-05-03 Thread 이희승 (Trustin Lee)
On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
 2011/5/2 Manik Surtani ma...@jboss.org:

 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

2011-05-03 Thread Sanne Grinovero
2011/5/3 이희승 (Trustin Lee) trus...@gmail.com:
 On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
 2011/5/2 Manik Surtani ma...@jboss.org:

 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

2011-05-03 Thread 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 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

2011-05-03 Thread 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  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

2011-05-03 Thread 이희승 (Trustin Lee)
On 05/03/2011 09:33 PM, Sanne Grinovero wrote:
 2011/5/3 이희승 (Trustin Lee) trus...@gmail.com:
 On 05/03/2011 05:08 AM, Sanne Grinovero wrote:
 2011/5/2 Manik Surtani ma...@jboss.org:

 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, ClassV 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

2011-05-02 Thread 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

2011-05-02 Thread Thomas P. Fuller
I like this solution.

T





From: Manik Surtani ma...@jboss.org
To: infinispan -Dev List infinispan-dev@lists.jboss.org
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

2011-05-02 Thread 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 sanne.grinov...@gmail.com
To: infinispan -Dev List infinispan-dev@lists.jboss.org
Sent: Mon, 2 May, 2011 21:08:47
Subject: Re: [infinispan-dev] [Pull Request] Modular Classloading Compatibility

2011/5/2 Manik Surtani ma...@jboss.org:

 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

2011-05-01 Thread 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

2011-04-29 Thread 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