]> source.dussan.org Git - aspectj.git/commitdiff
Weaver returns null instead of original bytes for unwoven classes
authorAlexander Kriegisch <Alexander@Kriegisch.name>
Wed, 7 Feb 2024 02:35:55 +0000 (09:35 +0700)
committerAlexander Kriegisch <Alexander@Kriegisch.name>
Wed, 7 Feb 2024 04:48:39 +0000 (11:48 +0700)
This change makes sense independently of #277, but also enables using

  - cp "my.jar;aspectjweaver.jar"
  -XX:+AllowArchivingWithJavaAgent
  -javaagent:aspectjweaver.jar

while creating a CDS archive. Afterward, the application can be run in
its woven state from the CDS archive even without '-javaagent', because
the byte code was archived in its woven state ("poor man's AJC"). See
https://github.com/eclipse-aspectj/aspectj/issues/277#issuecomment-1931142753
for details.

Fixes #277.

Signed-off-by: Alexander Kriegisch <Alexander@Kriegisch.name>
loadtime/src/main/java/org/aspectj/weaver/loadtime/Aj.java
loadtime/src/main/java/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java
loadtime/src/main/java/org/aspectj/weaver/loadtime/ClassPreProcessor.java
loadtime/src/main/java/org/aspectj/weaver/loadtime/ClassPreProcessorAgentAdapter.java
loadtime/src/main/java/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java
weaver/src/main/java/org/aspectj/weaver/tools/WeavingAdaptor.java
weaver/src/main/java/org/aspectj/weaver/tools/cache/SimpleCache.java

index 3c6ab2952cd66d5b3cd51109f7e266a26044cb6d..a8526ee55be8a1e84b3086a2531f73af0349a604 100644 (file)
@@ -75,41 +75,41 @@ public class Aj implements ClassPreProcessor {
        private final static String deleLoader2 = "jdk.internal.reflect.DelegatingClassLoader"; // On JDK11+
 
        @Override
-       public byte[] preProcess(String className, byte[] bytes, ClassLoader loader, ProtectionDomain protectionDomain) {
-               if (loader == null || className == null ||
-                       loader.getClass().getName().equals(deleLoader) || loader.getClass().getName().equals(deleLoader2)) {
+       public byte[] preProcess(String className, final byte[] bytes, ClassLoader classLoader, ProtectionDomain protectionDomain) {
+               if (classLoader == null || className == null ||
+                       classLoader.getClass().getName().equals(deleLoader) || classLoader.getClass().getName().equals(deleLoader2)) {
                        // skip boot loader, null classes (hibernate), or those from a reflection loader
-                       return bytes;
+                       return null;
                }
 
                if (loadersToSkip != null) {
                        // Check whether to reject it
-                       if (loadersToSkip.contains(loader.getClass().getName())) {
+                       if (loadersToSkip.contains(classLoader.getClass().getName())) {
 //                             System.out.println("debug: no weaver created for loader '"+loader.getClass().getName()+"'");
-                               return bytes;
+                               return null;
                        }
                }
 
                if (trace.isTraceEnabled())
-                       trace.enter("preProcess", this, new Object[] { className, bytes, loader });
+                       trace.enter("preProcess", this, new Object[] { className, bytes, classLoader });
                if (trace.isTraceEnabled())
-                       trace.event("preProcess", this, new Object[] { loader.getParent(), Thread.currentThread().getContextClassLoader() });
+                       trace.event("preProcess", this, new Object[] { classLoader.getParent(), Thread.currentThread().getContextClassLoader() });
 
                try {
-                       synchronized (loader) {
+                       synchronized (classLoader) {
 
                                if (SimpleCacheFactory.isEnabled()) {
-                                       byte[] cacheBytes= laCache.getAndInitialize(className, bytes,loader,protectionDomain);
+                                       byte[] cacheBytes= laCache.getAndInitialize(className, bytes, classLoader, protectionDomain);
                                        if (cacheBytes!=null){
                                                        return cacheBytes;
                                        }
                                }
 
-                               WeavingAdaptor weavingAdaptor = WeaverContainer.getWeaver(loader, weavingContext);
+                               WeavingAdaptor weavingAdaptor = WeaverContainer.getWeaver(classLoader, weavingContext);
                                if (weavingAdaptor == null) {
                                        if (trace.isTraceEnabled())
                                                trace.exit("preProcess");
-                                       return bytes;
+                                       return null;
                                }
                                try {
                                        weavingAdaptor.setActiveProtectionDomain(protectionDomain);
@@ -134,7 +134,7 @@ public class Aj implements ClassPreProcessor {
                        // would make sense at least in test f.e. see TestHelper.handleMessage()
                        if (trace.isTraceEnabled())
                                trace.exit("preProcess", th);
-                       return bytes;
+                       return null;
                } finally {
                        CompilationAndWeavingContext.resetForThread();
                }
index 9a10520755560c57c0e00533a287b077dac72df9..9d478633fec7a45dc51a1937a2c6ea4f7afbcb05 100644 (file)
@@ -569,7 +569,7 @@ public class ClassLoaderWeavingAdaptor extends WeavingAdaptor {
 
                        try {
                                byte[] newBytes = weaveClass(name, bytes, true);
-                               this.generatedClassHandler.acceptClass(name, bytes, newBytes);
+                               this.generatedClassHandler.acceptClass(name, bytes, newBytes == null ? bytes : newBytes);
                        } catch (IOException ex) {
                                trace.error("weaveAndDefineConceteAspects", ex);
                                error("exception weaving aspect '" + name + "'", ex);
index ab305cb32ea0182603464b5895f75e77460b92b6..5d9b74f8748f21bc5002d936802b3bc9f806ea20 100644 (file)
@@ -24,7 +24,18 @@ public interface ClassPreProcessor {
         */
        void initialize();
 
-       byte[] preProcess(String className, byte[] bytes, ClassLoader classLoader, ProtectionDomain protectionDomain);
+       /**
+        * @param className        the name of the class in the internal form of fully qualified class and interface names as
+        *                         defined in <i>The Java Virtual Machine Specification</i>. For example,
+        *                         <code>"java/util/List"</code>.
+        * @param bytes            the input byte buffer in class file format - must not be modified
+        * @param classLoader      the defining loader of the class to be transformed, may be {@code null} if the bootstrap
+        *                         loader
+        * @param protectionDomain the protection domain of the class being defined or redefined
+        *
+        * @return a well-formed class file buffer (weaving result), or {@code null} if no weaving was performed
+        */
+       byte[] preProcess(String className, final byte[] bytes, ClassLoader classLoader, ProtectionDomain protectionDomain);
 
        void prepareForRedefinition(ClassLoader loader, String className);
 }
index 2a778585e24950f30873e6f03598d9dc9869e09b..45de1f14e0c399c73c4a3944a8e95f63f8e3042d 100644 (file)
@@ -43,7 +43,7 @@ public class ClassPreProcessorAgentAdapter implements ClassFileTransformer {
         */
        @Override
        public byte[] transform(ClassLoader loader, String className, Class<?> classBeingRedefined, ProtectionDomain protectionDomain,
-                       byte[] bytes) throws IllegalClassFormatException {
+                       final byte[] bytes) throws IllegalClassFormatException {
                if (classBeingRedefined != null) {
                        System.err.println("INFO: (Enh120375):  AspectJ attempting reweave of '" + className + "'");
                        classPreProcessor.prepareForRedefinition(loader, className);
index 6334120f10ee6ea2fa6cb4284b976e57a4f80f70..d363798500b3e534c6a99678a486de8dc4136ca3 100644 (file)
@@ -136,6 +136,8 @@ public class WeavingURLClassLoader extends ExtensibleURLClassLoader implements W
 
                        try {
                                b = adaptor.weaveClass(name, b, false);
+                               if (b == null)
+                                       b = orig;
                        } catch (AbortException ex) {
                                trace.error("defineClass", ex);
                                throw ex;
@@ -149,7 +151,7 @@ public class WeavingURLClassLoader extends ExtensibleURLClassLoader implements W
                try {
                        clazz= super.defineClass(name, b, cs);
                } catch (Throwable th) {
-                       trace.error("Weaving class problem. Original class has been returned. The error was caused because of: " + th, th);
+                       trace.error("Weaving class problem. Original class has been returned. Error cause: " + th, th);
                        clazz= super.defineClass(name, orig, cs);
                }
                if (trace.isTraceEnabled()) {
index 3ce3eef6ea739d853876393bff4bb1bbd34d200a..41f19db557fc6c12222c8e8284c8574b2aadbf43 100644 (file)
@@ -318,26 +318,30 @@ public class WeavingAdaptor implements IMessageContext {
        /**
         * Weave a class using aspects previously supplied to the adaptor.
         *
-        * @param name the name of the class
-        * @param bytes the class bytes
-        * @param mustWeave if true then this class *must* get woven (used for concrete aspects generated from XML)
-        * @return the woven bytes
-        * @exception IOException weave failed
+        * @param name      the name of the class in the internal form of fully qualified class and interface names as defined
+        *                  in <i>The Java Virtual Machine Specification</i>. For example, <code>"java/util/List"</code>.
+        * @param bytes     the input byte buffer in class file format - must not be modified
+        * @param mustWeave if true then this class <i>must</i> get woven (used for concrete aspects generated from XML)
+        *
+        * @return a well-formed class file buffer (the weaving result), or {@code null} if no weaving was performed
+        *
+        * @throws IOException weave failed
         */
-       public byte[] weaveClass(String name, byte[] bytes, boolean mustWeave) throws IOException {
+       public byte[] weaveClass(String name, final byte[] bytes, boolean mustWeave) throws IOException {
                if (trace == null) {
                        // Pr231945: we are likely to be under tomcat and ENABLE_CLEAR_REFERENCES hasn't been set
                        System.err
                                        .println("AspectJ Weaver cannot continue to weave, static state has been cleared.  Are you under Tomcat? In order to weave '"
                                                        + name
                                                        + "' during shutdown, 'org.apache.catalina.loader.WebappClassLoader.ENABLE_CLEAR_REFERENCES=false' must be set (see https://bugs.eclipse.org/bugs/show_bug.cgi?id=231945).");
-                       return bytes;
+                       return null;
                }
                if (weaverRunning.get()) {
                        // System.out.println("AJC: avoiding re-entrant call to transform " + name);
-                       return bytes;
+                       return null;
                }
                try {
+                       byte[] newBytes = null;
                        weaverRunning.set(true);
                        if (trace.isTraceEnabled()) {
                                trace.enter("weaveClass", this, new Object[] { name, bytes });
@@ -347,7 +351,7 @@ public class WeavingAdaptor implements IMessageContext {
                                if (trace.isTraceEnabled()) {
                                        trace.exit("weaveClass", false);
                                }
-                               return bytes;
+                               return null;
                        }
 
                        boolean debugOn = !messageHandler.isIgnoring(Message.DEBUG);
@@ -360,15 +364,14 @@ public class WeavingAdaptor implements IMessageContext {
 
                                                // Determine if we have the weaved class cached
                                                CachedClassReference cacheKey = null;
-                                               final byte[] original_bytes = bytes;
                                                if (cache != null && !mustWeave) {
-                                                       cacheKey = cache.createCacheKey(name, original_bytes);
-                                                       CachedClassEntry entry = cache.get(cacheKey, original_bytes);
+                                                       cacheKey = cache.createCacheKey(name, bytes);
+                                                       CachedClassEntry entry = cache.get(cacheKey, bytes);
                                                        if (entry != null) {
                                                                // If the entry has been explicitly ignored
                                                                // return the original bytes
                                                                if (entry.isIgnored()) {
-                                                                       return bytes;
+                                                                       return null;
                                                                }
                                                                return entry.getBytes();
                                                        }
@@ -382,7 +385,12 @@ public class WeavingAdaptor implements IMessageContext {
                                                if (debugOn) {
                                                        debug("weaving '" + name + "'");
                                                }
-                                               bytes = getWovenBytes(name, bytes);
+                                               newBytes = getWovenBytes(name, bytes);
+                                               // TODO: Is this OK performance-wise?
+                                               if (Arrays.equals(bytes, newBytes)) {
+                                                       // null means unchanged in java.lang.instrument.ClassFileTransformer::transform
+                                                       newBytes = null;
+                                               }
                                                // temporarily out - searching for @Aspect annotated types is a slow thing to do - we should
                                                // expect the user to name them if they want them woven - just like code style
                                                // } else if (shouldWeaveAnnotationStyleAspect(name, bytes)) {
@@ -405,10 +413,10 @@ public class WeavingAdaptor implements IMessageContext {
                                                if (cacheKey != null) {
                                                        // If no transform has been applied, mark the class
                                                        // as ignored.
-                                                       if (Arrays.equals(original_bytes, bytes)) {
-                                                               cache.ignore(cacheKey, original_bytes);
+                                                       if (newBytes == null) {
+                                                               cache.ignore(cacheKey, bytes);
                                                        } else {
-                                                               cache.put(cacheKey, original_bytes, bytes);
+                                                               cache.put(cacheKey, bytes, newBytes);
                                                        }
                                                }
                                        } else if (debugOn) {
@@ -422,9 +430,9 @@ public class WeavingAdaptor implements IMessageContext {
                        }
 
                        if (trace.isTraceEnabled()) {
-                               trace.exit("weaveClass", bytes);
+                               trace.exit("weaveClass", newBytes);
                        }
-                       return bytes;
+                       return newBytes;
                } finally {
                        weaverRunning.remove();
                }
index 4bbf8d9c9ba4a13ddd6b68b67efe03c68efb755a..0cf759e73f221a5ab8aa4ebaf8578cebd678c546 100644 (file)
@@ -72,6 +72,7 @@ public class SimpleCache {
                byte[] res = get(classname, bytes);
 
                if (Arrays.equals(SAME_BYTES, res)) {
+                       // TODO: Should we return null (means "not transformed") in this case?
                        return bytes;
                } else {
                        if (res != null) {