openjpa-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rmannibu...@apache.org
Subject [openjpa] 01/02: OPENJPA-2817 dropping _transforming from PCClassFileTransformer and replace it by a minimal exclusion list
Date Thu, 23 Jul 2020 19:17:40 GMT
This is an automated email from the ASF dual-hosted git repository.

rmannibucau pushed a commit to branch OPENJPA-2817_PCClassFileTransformer-exclusions
in repository https://gitbox.apache.org/repos/asf/openjpa.git

commit fd70273f246255e86411bc4ed738860bd5cdc8f6
Author: Romain Manni-Bucau <rmannibucau@gmail.com>
AuthorDate: Tue Jul 14 19:38:48 2020 +0200

    OPENJPA-2817 dropping _transforming from PCClassFileTransformer and replace it by a minimal
exclusion list
---
 .../openjpa/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 d67fb48..45bc953 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 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)
         throws IllegalClassFormatException {
@@ -116,17 +119,43 @@ public class PCClassFileTransformer
         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 @@ 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);
@@ -170,7 +199,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));
@@ -180,8 +208,10 @@ public class PCClassFileTransformer
     /**
      * 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 @@ public class PCClassFileTransformer
 
         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) {


Mime
View raw message