rmannibucau edited a comment on pull request #64: URL: https://github.com/apache/openjpa/pull/64#issuecomment-657785926
@jgrassel 1. hmm, depends once again the env, with a javaagent it will be triggered on a new MyEntity() but on other cases it will not. And for javaagent this is not an issue since the context is protected properly by the classloader normally. 2. this is a setup bug we shouldn't have. Correct fix is probably to prevent openjpa to be loaded from the temp loader (likely container integration) and/or to skip openjpa stack in transform (we can skip more actually, see https://github.com/apache/tomee/blob/master/container/openejb-core/src/main/java/org/apache/openejb/util/classloader/URLClassLoaderFirst.java#L207 for a list example). A toggle is a quick and dirty fix but does not solve the cause of this issue IMHO. The threadlocal generalizes this workaround but still does not tackle the source of the issue. I'd like to fix it right on master at least. 3. The transformer is executed in a secured context by design/contract so this must be an issue with the env so part of 2 (the exclusion list in transform) and potentially server classloader enhancement support. What the boolean - with a threadlocal or not - enables is to not enhance some classes - this is why sometimes we can have "missing metadata" error in some setup because if an enhancement triggers the load of another entity, this last one is not enhanced - depends the tmploader which can be the same as app loader when using an agent to speed up the execution/boot. FYI: dropping the boolean the build still works fine so guess we should work forward the exclusion list. What about something like that: diff --git a/openjpa-kernel/src/main/java/org/apache/openjpa/enhance/PCClassFileTransformer.java b/openjpa-kernel/src/main/java/org/apache/openjpa/enhance/PCClassFileTransformer.java index 872d413a4..1126762ef 100644 --- a/openjpa-kernel/src/main/java/org/apache/openjpa/enhance/PCClassFileTransformer.java +++ b/openjpa-kernel/src/main/java/org/apache/openjpa/enhance/PCClassFileTransformer.java @@ -55,7 +55,6 @@ public class PCClassFileTransformer private final ClassLoader _tmpLoader; private final Log _log; private final Set _names; - private boolean _transforming = false; /** * Constructor. @@ -107,6 +106,9 @@ public class PCClassFileTransformer _log.info(_loc.get("runtime-enhance-pcclasses")); } + // this must be called under a proper locking, this is guaranteed by either + // a correct concurrent classloader with transformation support OR + // a mono-threaded access guarantee (build, deploy time enhancements). @Override public byte[] transform(ClassLoader loader, String className, Class redef, ProtectionDomain domain, byte[] bytes) @@ -115,16 +117,13 @@ public class PCClassFileTransformer return null; // JDK bug -- OPENJPA-1676 - if (className == null) { + if (className == null || + className.startsWith("org/apache/openjpa/") || + className.startsWith("java/") || + className.startsWith("javax/") || + className.startsWith("jakarta/")) { return null; } - // prevent re-entrant calls, which can occur if the enhancing - // loader is used to also load OpenJPA libraries; this is to prevent - // recursive enhancement attempts for internal openjpa libraries - if (_transforming) - return null; - - _transforming = true; return transform0(className, redef, bytes); } @@ -151,7 +150,7 @@ public class PCClassFileTransformer try { PCEnhancer enhancer = new PCEnhancer(_repos.getConfiguration(), new Project().loadClass(new ByteArrayInputStream(bytes), - _tmpLoader), _repos); + _tmpLoader), _repos, _tmpLoader); enhancer.setAddDefaultConstructor(_flags.addDefaultConstructor); enhancer.setEnforcePropertyRestrictions (_flags.enforcePropertyRestrictions); @@ -172,7 +171,6 @@ public class PCClassFileTransformer throw (IllegalClassFormatException) t; throw new GeneralException(t); } finally { - _transforming = false; if (returnBytes != null && _log.isTraceEnabled()) _log.trace(_loc.get("runtime-enhance-complete", className, bytes.length, returnBytes.length)); @@ -186,9 +184,9 @@ public class PCClassFileTransformer if (redef != null) { Class[] intfs = redef.getInterfaces(); for (int i = 0; i < intfs.length; i++) - if (PersistenceCapable.class.getName(). - equals(intfs[i].getName())) - return Boolean.valueOf(!isEnhanced(bytes)); + if (PersistenceCapable.class.getName() + .equals(intfs[i].getName())) + return false; return null; } @@ -198,8 +196,6 @@ public class PCClassFileTransformer return null; } - if (clsName.startsWith("java/") || clsName.startsWith("javax/")) - return null; if (isEnhanced(bytes)) return Boolean.FALSE; ---------------------------------------------------------------- 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
