[infinispan-issues] [JBoss JIRA] Commented: (ISPN-1096) Rework application class loading to be more friendly to modular environments

[Pete Muir (JIRA)]

    [ https://issues.jboss.org/browse/ISPN-1096?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12611210#comment-12611210 ] 

Pete Muir commented on ISPN-1096:
---------------------------------

The API for this is complete, and almost all code is using the new approach. However tying this into the marshaller framework is pretty messy right now and I can't see a tidy way to do it. 

* JBoss Marshaller requires a classloader configured for an instance. This is because the marshaller framework hands out objects to which you can write/read objects
* Infinispan configures a marshaller per thread

I have looked at various options for wiring this in. Without substantially changing the marshalling architecture (so that we have a marshaller per named cache instance per thread, which not only requires changing the marshalling architecture, but also the DI to account for named cache instances) the best way to handle this is to set the TCCL before all use of the marshaller and then back out the TCCL set after use. It's fairly straightforward to use the InvocationContext to get the right CL set on the cache instance to the places which use the marshaller. However, I am reluctant to use this approach because of (a) the inherent issues associated with setting the TCCL (it's very easy to end up with yourself not unsetting it) and (b) because this represents dreadful separation of concerns (this TCCL set code will be sprinkled across the code base) and (c) it's kind of crazy to do it this way, we do actually know the right classloader to use.

For this reason, I would propose that we:

a) close this issue out and create a new one for handling marshalling
b) schedule the one relating to marshalling to 5.1 and "do this properly" with a re-architected marshaller taking into account the classloading.

I don't think do a substantial rearch on the marshaller is sensible given the timeframe for 5.0, furthermore the user facing API is in place in 5.0, the marshaller changes will be SPI level only, so possible to defer.

> Rework application class loading to be more friendly to modular environments
> ----------------------------------------------------------------------------
>
>                 Key: ISPN-1096
>                 URL: https://issues.jboss.org/browse/ISPN-1096
>             Project: Infinispan
>          Issue Type: Feature Request
>          Components: Core API
>    Affects Versions: 4.2.1.FINAL
>            Reporter: Pete Muir
>            Assignee: Pete Muir
>            Priority: Blocker
>              Labels: ClassLoader, Modularity, OSGi
>             Fix For: 5.0.0.CR7, 5.0.0.FINAL
>
>
> There are 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().
> This has been addressed as a separate issue.
> 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).

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira