From fd70273f246255e86411bc4ed738860bd5cdc8f6 Mon Sep 17 00:00:00 2001 From: Romain Manni-Bucau Date: Tue, 14 Jul 2020 19:38:48 +0200 Subject: [PATCH 1/2] OPENJPA-2817 dropping _transforming from PCClassFileTransformer and replace it by a minimal exclusion list --- .../enhance/PCClassFileTransformer.java | 78 +++++++++++++------ 1 file changed, 53 insertions(+), 25 deletions(-) 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 d67fb48a11..45bc953374 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. @@ -64,7 +63,8 @@ public class PCClassFileTransformer * @param opts enhancer configuration options * @param loader temporary class loader for loading intermediate classes */ - public PCClassFileTransformer(MetaDataRepository repos, Options opts, ClassLoader loader) { + public PCClassFileTransformer(MetaDataRepository repos, Options opts, + ClassLoader loader) { this(repos, toFlags(opts), loader, opts.removeBooleanProperty ("scanDevPath", "ScanDevPath", false)); } @@ -105,6 +105,9 @@ public PCClassFileTransformer(MetaDataRepository repos, PCEnhancer.Flags flags, _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) throws IllegalClassFormatException { @@ -116,17 +119,43 @@ public byte[] transform(ClassLoader loader, String className, Class redef, Prote if (className == null) { 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); } + // very simplified flavor of + // @apache/tomee > + // container/openejb-core > + // org/apache/openejb/util/classloader/URLClassLoaderFirst.java#L207 + private boolean isExcluded(final String className) { + if ( // api + className.startsWith("javax/") || + className.startsWith("jakarta/") || + // openjpa dependencies + className.startsWith("serp/") || + // jvm + className.startsWith("java/") || + className.startsWith("sun/") || + className.startsWith("jdk/")) { + return true; + } + // it is faster to use substring when multiple tests are needed + if (className.startsWith("org/apache/")) { + final String sub = className.substring("org/apache/".length()); + if (sub.startsWith("openjpa/") || + sub.startsWith("commons/")) { + return true; + } + } + if (className.startsWith("com/")) { + final String sub = className.substring("com/".length()); + if (sub.startsWith("oracle/") || sub.startsWith("sun/")) { // jvm + return true; + } + } + return false; + } + /** * We have to split the transform method into two methods to avoid * ClassCircularityError when executing method using pure-JIT JVMs @@ -149,7 +178,7 @@ private byte[] transform0(String className, Class redef, byte[] bytes) 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); @@ -170,7 +199,6 @@ private byte[] transform0(String className, Class redef, byte[] bytes) 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)); @@ -180,8 +208,10 @@ private byte[] transform0(String className, Class redef, byte[] bytes) /** * Return whether the given class needs enhancement. */ - private Boolean needsEnhance(String clsName, Class redef, byte[] bytes) { - if (redef != null && PersistenceCapable.class.isAssignableFrom(redef)) { + private Boolean needsEnhance(String clsName, Class redef, byte[] bytes) { + final boolean excluded = isExcluded(clsName); + + if (!excluded && redef != null && PersistenceCapable.class.isAssignableFrom(redef)) { // if the original class is already enhanced (implements PersistenceCapable) // then we don't need to do any further processing. return null; @@ -189,32 +219,30 @@ private Boolean needsEnhance(String clsName, Class redef, byte[] bytes) { if (_names != null) { if (_names.contains(clsName.replace('/', '.'))) - return Boolean.valueOf(!isEnhanced(bytes)); - return null; - } - - if (clsName.startsWith("java/") || clsName.startsWith("javax/") || clsName.startsWith("jakarta/")) { + return !isEnhanced(bytes); + if (excluded) { + return false; + } return null; } - if (isEnhanced(bytes)) { - return Boolean.FALSE; - } + if (excluded || isEnhanced(bytes)) + return false; try { - Class c = Class.forName(clsName.replace('/', '.'), false, + Class c = Class.forName(clsName.replace('/', '.'), false, _tmpLoader); if (_repos.getMetaData(c, null, false) != null) - return Boolean.TRUE; + return true; return null; } catch (ClassNotFoundException cnfe) { // cannot load the class: this might mean that it is a proxy // or otherwise inaccessible class which can't be an entity - return Boolean.FALSE; + return false; } catch (LinkageError cce) { // this can happen if we are loading classes that this // class depends on; these will never be enhanced anyway - return Boolean.FALSE; + return false; } catch (RuntimeException re) { throw re; } catch (Throwable t) { From 1c14bc28cb399e32afba5e59d4b23c6eb6ad3067 Mon Sep 17 00:00:00 2001 From: Romain Manni-Bucau Date: Tue, 14 Jul 2020 20:25:00 +0200 Subject: [PATCH 2/2] excluding asm too --- .../org/apache/openjpa/enhance/PCClassFileTransformer.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 45bc953374..f0ac151574 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 @@ -143,7 +143,8 @@ private boolean isExcluded(final String className) { if (className.startsWith("org/apache/")) { final String sub = className.substring("org/apache/".length()); if (sub.startsWith("openjpa/") || - sub.startsWith("commons/")) { + sub.startsWith("commons/") || + sub.startsWith("xbean/")) { return true; } }