Custom class loading strategy for cglib and javassist proxies #159

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@unintended

Fixes https://hibernate.onjira.com/browse/HHH-5962 Issue.
The issue was that Hibernate's classloader uses the classloader of mapped classes to load its own classes (i.e. org.hibernate.* etc) after creating the proxy. It works well if the classloader of the enhanced classes is the same as the Hibernate's one but it fails if this classloader knows nothing about the hibernate classes (e.g. if the mapped classes are dynamically loaded from another bundle).

I've been testing the patched Hibernate with my OSGi project for a few months and it works pretty well.

@sebersole

CGLIB support is no more.

But in terms of JavassistLazyInitializer, why create a whole new ClassLoader instead of just using persistentClass.getClassLoader()?

Owner

It's related to this issue: https://hibernate.onjira.com/browse/HHH-5962

This new classloader combines two classloaders - one of persistentClass and one of this.getClass(). They are NOT the same if the system runs in OSGi mode, where hibernate core is located in one bundle (with it's own classloader) and the persistent classes are located in other bundles (with their own classloaders). So this classloader makes possible to load all org.hibernate.* and javassist.* classes using hibernate bundle's own platformClassLoader and load persistent classes using their bundle's classloaders.

Probably there is a reason to check if this is really needed and the classloaders are not equal, i.e.
if (!persistentClass.getClassLoader().equals(getClass().getClassLoader()) { // use complex classloader }

Sure, but what I am asking you is why it needs the "Hibernate classloader" (getClass().getClassLoader())? AFAIU, all the Hibernate classes it would need are already passed in as Class references.

What is the use case that led you to believe you need both?

Also, would really like to see a test case for this to make sure it does not regress later.

Not saying you are wrong btw. Really just asking because it seems to me you would not need both.

Anyway, 4.1.1 is scheduled for tomorrow. If there is any chance you could get this done today (central US time) I will get this into 4.1.1. If not, it will have to wait for 4.1.2 or 5.0

Owner

I don't have enough time today to reinvestigate the issue to the deeps and create testcases for it (it looks like it's no so simple to reproduce osgi-like environment in TC) to give you some points for this patch but it's definitely right thing. You see I had to create an opensource hibernate-osgi fork with only this modification to make possible to use it in our project.

I can try to explain the reason for accessing both classloaders:
Without this patch Hibernate assumes and requires that all persistent classes' classloaders have access to hibernate and proxy classes. But in some environments it's not logically right and persistent classes may have another classloader that knows nothing about hibernate imports.

At some point it does the following JavassistProxyFactory.postInstantiate():
factory = JavassistLazyInitializer.getProxyFactory( persistentClass, this.interfaces );
with persistentClass set to my pojo class(loaded with another 'hibernate-less' classloader) and set of interfaces to support: [org.hibernate.proxy.HibernateProxy]

During this call creating the factory it uses my pojo's 'hibernate-less' classloader to load the following classes that are not expected to be known to it (I just put a breakpoint into my findClass to catch them):
org.hibernate.proxy.HibernateProxy
javassist.util.proxy.ProxyObject
javassist.util.proxy.MethodHandler
javassist.util.proxy.MethodFilter
...

To make possible to put pojo classes into some clean 'bundle' and make them not to depend on Hibernate and javassist I created a new ClassLoader with parent set to persistentClass's classloader and override findClass(). This findClass() uses platformClassLoader to load everything that wasn't already loaded and found with parent.

If I have time to reinvestigate it again I may provide more detailed explanations, but not today.

@brmeyer
Member
brmeyer commented Feb 21, 2013

Closing this. CGLIBLazyInitializer no longer exists and any other OSGi ClassLoading issues are or will be addressed by https://hibernate.onjira.com/browse/HHH-7527

@brmeyer brmeyer closed this Feb 21, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment