jgrassel commented on pull request #65: URL: https://github.com/apache/openjpa/pull/65#issuecomment-658350601
@rmannibucau That optimization sounds like a violation of the PersistenceUnitInfo contract: ``` /** * Return a new instance of a ClassLoader that the provider may * use to temporarily load any classes, resources, or open * URLs. The scope and classpath of this loader is exactly the * same as that of the loader returned by * PersistenceUnitInfo.getClassLoader. None of the classes loaded * by this class loader will be visible to application * components. The provider may only use this ClassLoader within * the scope of the createContainerEntityManagerFactory call. * @return temporary ClassLoader with same visibility as current * loader */ public ClassLoader getNewTempClassLoader(); ``` It specifically states `None of the classes loaded by this class loader will be visible to application components.` -- that is impossible if the Temporary CL is the same object instance as the App CL. Anyways, the `_transforming` boolean was put in there to catch a reentrant call triggered during enhancement, which could only occur if the JPA impl classes are loaded by the same CL as the entity classes. See this flow: ``` 1. MyEntity is loaded by the ClassLoader for the first time. Bytecode is fetched, and passed to the enhancer for transformation (if necessary) 2. Control enters the OpenJPA Enhancer code. (_transforming is set true) 3. While in the OpenJPA Enhancer code, one of the OpenJPA impl types needs to be loaded by the ClassLoader. This triggers the ClassLoader (which this thread has a lock on, as back then CLer access was synchronized) to load that OpenJPA impl type's bytecode and calls the enhancer (the CL just knows this is a class that it is loading, it cannot distinguish an entity class from a non-entity class) 4. Control re-enters the OpenJPA Enhancer code 5. Because this reentrancy can only happen if a class is loaded while processing the enhancement logic (step 3), the class being requested for transformation could only be an openjpa type, which is never going to be enhanced. So return null (no transform) -- this condition is detected by the `_transforming` field set in (step 2). 6. Transformer returns with no change, CLer finishes loading the OpenJPA type 7. Control returns from the CLer back to the transformer 8. Transformer finishes transforming the class, and sets _transforming to false 9. Control returns to ClassLoader, which loads the transformed bytecode 10. Done, profit! ``` Now, because ClassLoaders now permit concurrent access among multiple threads, that _transforming boolean has to be thread context aware -- it should allow any number of threads to use the transformer, but the same thread would not be calling a classload and transform for any type other than a JPA provider impl type. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
