ResourceFactories and Segmented ClassLoaders

[madjam]

With recent changes that introduce ResourceFactories, I'm having trouble creating a custom resource. The custom resource and its classes are defined in a bundle A and Atomix classes are in bundle B. They don't share class loaders.

As a result deserialization code in https://github.com/atomix/atomix/blob/master/resource/src/main/java/io/atomix/resource/ResourceType.java#L83 fails with a ClassNotFoundException

@kuujo: Is it possible to have this be compatible with a OSGi environment where there are multiple class loaders?

[jhalterman]

I think I've worked around this in the past by using the Thread context classloader for loading classes that need to be visible to different OSGi bundles. @madjam What do you think?

Edit: A bit of reading and the above seems like a bad idea.

A few other potential options might be allowing the ClassLoader to be specified in the API as part of configuration, or using a bridge ClassLoader internally, similar to what Guice does, bridging the visibility of loaded classes between the library and the client's classes.

[madjam]

@jhalterman That is a good suggestion. The trouble I see in this context is that the Thread doing the deserialization is the thread that is created here: https://github.com/atomix/copycat/blob/master/server/src/main/java/io/atomix/copycat/server/CopycatServer.java#L969. And that thread's ClassLoader is not aware of the custom ResourceFactory (a quick test confirms this)

[jhalterman]

Neil Bartlett seems to have the authoritative solutions we can choose from here:

http://njbartlett.name/2010/08/30/osgi-readiness-loading-classes.html

(scroll down to the solutions section).

I forgot that the bridge classloader idea still requires the user to pass a reference to the Class they want to load, which is how Guice obtains the user's classloader reference.

[madjam]

This is a good list.
Options 2 and 3 seen reasonable to me. Between the two I'm leaning towards option 2.
What do you think?

[jhalterman]

@madjam I agree with option 2. @kuujo ?

[jhalterman]

@madjam Are you bundling Atomix? Maybe it would be useful to add proper OSGI support to the Atomix modules? Also, can you advise on how to create a tiny OSGI bundle that replicates your problem so we can verify the fix, whatever it ends up being?

Edit: just filed #157.

[madjam]

It would be great to have OSGi support.
I currently use the shade maven plugin to create a uber jar of all dependencies that are not bundled. This uber jar is bundled using the maven bundle plugin.

I have a provisional fix for this issue. It involves adding registerClassLoader and getClassLoader methods to the Serializer. It's likely there is a better way to manage the class loader registry. But the change I have does work. I'll submit a pull request and you and @kuujo can chime in.

[kuujo]

Alright... I'm working on a solution for the ResourceFactory change also...

[kuujo]

@madjam that would be great actually that's exactly what I was thinking before I read your comment :-)
Better you fix it than me to ensure the issue is resolved. We can change that writeObject/readObject implementation to use serializer.readObject(buffer) instead of Class.forName as well. Class is a serializable type in Catalyst, it just needs to use the proper class loader... unless you're going about it another way that will work.

[kuujo]

Nevermind I think you're saying the readObject method can get the ClassLoader from the Serializer. That seems fine too. But I wonder if it should be moved into the ClassSerializer itself rather than doing it at the Atomix level:
https://github.com/atomix/catalyst/blob/master/serializer/src/main/java/io/atomix/catalyst/serializer/lang/ClassSerializer.java#L39
That code can get the ClassLoader and load the class there. Serializers at the Atomix level should just not be doing any class loading.

[madjam]

Thanks @kuujo. My thoughts were in line with your suggestion. I opened a PR for this: atomix/catalyst#40

[madjam]

#158 is the Atomix PR to simply the serialization logic in ResourceType

[madjam]

Resolved via atomix/catalyst#40 and #158