Some responses to Brett's questions:

Can you clarify what you mean by "unpublish the EMF"? For use with Aries JPA container-managed JPA, closing the EMF makes sense (I think...), but I'm tripping over what you mean by unpublishing.

Aries JPA publishes the EntityManagerFactory as an OSGi service. If any of the prerequisites of the service go away this OSGi services is unpublished. This gives all clients of the service the chance to cleanup any references to the EMF. This is very important to allow that the classloader can then be cleaned up.
In Aries JPA blueprint and DS support I took care that the EntityManager instances are only created per request. So if the EMF goes away then all EntityManagers will be closed as soon as all currently running requests are processed. This is also part of making sure we clean up as soon as possible.

Aries JPA container tracks the opened EntityManagers when the EMF is closed it first waits some time for them to close themselves and if that does not happen it forcefully closes them.

Bram Pouwelse I think it would be best to use the ClassLoader from the requestingBundle in the OsgiPersistenceProvider instead of creating a new ClassLoader that is mashing the ClassLoaders of all bundles using Hibernate in one OsgiClassLoader. By using the ClassLoader from the requestingBundle and not using the OsgiClassLoader there is no need track bundle events to cleanup.

As discussed on https://github.com/hibernate/hibernate-orm/pull/958, it's a great thought, but misses some nuances that still necessitate the need for the combined OsgiClassLoader. IIRC, loading resources provided by hibernate-entitymanager/hibernate-core (schemas, etc.) needed this approach. There may have been other reasons, but the specifics escape me. Explicitly setting it up this way is more consistent, rather than requiring the "requesting bundle" to ensure it's importing all packages necessary to make that work...

Yes. Unfortunately there are some cases where a combined classloader must be used. We should make sure though to only use it for those special cases.

This was in regards to container-managed JPA. Originally, the thought here was the same as the comment as above. But, if Aries really is setting the PersistenceUnitInfo.getClassLoader() up that way, maybe we'd be ok. The only piece that worries me here is "the classloader of the bundle that contains the persistence.xml", implying only hibernate-entitymanager, but missing hibernate-core. In theory that sounds ok (since core packages are imported in em), but again, nuances...

The classloader set by Aries JPA does not necessarily have access to hibernate packages. After all the persistence bundle might only have references to the JPA API and not know about hibernate at all.

Christian Schneider For application managed mode I think the ThreadContextClassloader could make sense. If the user knows that he has to set it berfore calling Persistence.createEntityManagerFactory then it should be safe.

-1 from me. My hope was to make this work OOTB as much as possible through automated-bootstrapping and OSGi services, rather than requiring workarounds.

I agree that providing some OOTB support would be convenient.

Bram Pouwelse One can workaround this issue by importing the org.hibernate.proxy and javassist.util.proxy packages in the bundle bundle containing the entity class.
I've also did a quick test changing the org.hibernate.proxy.pojo.javassist.JavassistProxyFactory#buildJavassistProxyFactory method to use a ClassLoader that aggregates two ClassLoaders (the Hibernate ClassLoader and the ClassLoader of the entity class). This resolves the issue and I don't see any ClassLoader issues.

That could indeed help. The only issue here is to make sure this classloader is also cleaned up when the persistence bundle goes down or is refreshed. The good thing with the current solution of manually importing the packages is that the persistence bundle can be freely updated. As long as this works with the combined classloader it would be great.