diff --git a/loadtime/src/main/java/org/aspectj/weaver/loadtime/Aj.java b/loadtime/src/main/java/org/aspectj/weaver/loadtime/Aj.java index 3c6ab2952c..a8526ee55b 100644 --- a/loadtime/src/main/java/org/aspectj/weaver/loadtime/Aj.java +++ b/loadtime/src/main/java/org/aspectj/weaver/loadtime/Aj.java @@ -75,41 +75,41 @@ public void initialize() { 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 byte[] preProcess(String className, byte[] bytes, ClassLoader loader, Pro // 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(); } diff --git a/loadtime/src/main/java/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java b/loadtime/src/main/java/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java index 9a10520755..9d478633fe 100644 --- a/loadtime/src/main/java/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java +++ b/loadtime/src/main/java/org/aspectj/weaver/loadtime/ClassLoaderWeavingAdaptor.java @@ -569,7 +569,7 @@ private boolean weaveAndDefineConceteAspects() { 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); diff --git a/loadtime/src/main/java/org/aspectj/weaver/loadtime/ClassPreProcessor.java b/loadtime/src/main/java/org/aspectj/weaver/loadtime/ClassPreProcessor.java index ab305cb32e..5d9b74f874 100644 --- a/loadtime/src/main/java/org/aspectj/weaver/loadtime/ClassPreProcessor.java +++ b/loadtime/src/main/java/org/aspectj/weaver/loadtime/ClassPreProcessor.java @@ -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 The Java Virtual Machine Specification. For example, + * "java/util/List". + * @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); } diff --git a/loadtime/src/main/java/org/aspectj/weaver/loadtime/ClassPreProcessorAgentAdapter.java b/loadtime/src/main/java/org/aspectj/weaver/loadtime/ClassPreProcessorAgentAdapter.java index 2a778585e2..45de1f14e0 100644 --- a/loadtime/src/main/java/org/aspectj/weaver/loadtime/ClassPreProcessorAgentAdapter.java +++ b/loadtime/src/main/java/org/aspectj/weaver/loadtime/ClassPreProcessorAgentAdapter.java @@ -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); diff --git a/loadtime/src/main/java/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java b/loadtime/src/main/java/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java index 6334120f10..d363798500 100644 --- a/loadtime/src/main/java/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java +++ b/loadtime/src/main/java/org/aspectj/weaver/loadtime/WeavingURLClassLoader.java @@ -136,6 +136,8 @@ protected Class defineClass(String name, byte[] b, CodeSource cs) throws IOExcep 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 @@ protected Class defineClass(String name, byte[] b, CodeSource cs) throws IOExcep 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()) { diff --git a/weaver/src/main/java/org/aspectj/weaver/tools/WeavingAdaptor.java b/weaver/src/main/java/org/aspectj/weaver/tools/WeavingAdaptor.java index 3ce3eef6ea..41f19db557 100644 --- a/weaver/src/main/java/org/aspectj/weaver/tools/WeavingAdaptor.java +++ b/weaver/src/main/java/org/aspectj/weaver/tools/WeavingAdaptor.java @@ -318,26 +318,30 @@ protected Boolean initialValue() { /** * 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 The Java Virtual Machine Specification. For example, "java/util/List". + * @param bytes the input byte buffer in class file format - must not be modified + * @param mustWeave if true then this class must 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 byte[] weaveClass(String name, byte[] bytes, boolean mustWeave) throws IO if (trace.isTraceEnabled()) { trace.exit("weaveClass", false); } - return bytes; + return null; } boolean debugOn = !messageHandler.isIgnoring(Message.DEBUG); @@ -360,15 +364,14 @@ public byte[] weaveClass(String name, byte[] bytes, boolean mustWeave) throws IO // 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 byte[] weaveClass(String name, byte[] bytes, boolean mustWeave) throws IO 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 byte[] weaveClass(String name, byte[] bytes, boolean mustWeave) throws IO 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 byte[] weaveClass(String name, byte[] bytes, boolean mustWeave) throws IO } if (trace.isTraceEnabled()) { - trace.exit("weaveClass", bytes); + trace.exit("weaveClass", newBytes); } - return bytes; + return newBytes; } finally { weaverRunning.remove(); } diff --git a/weaver/src/main/java/org/aspectj/weaver/tools/cache/SimpleCache.java b/weaver/src/main/java/org/aspectj/weaver/tools/cache/SimpleCache.java index 4bbf8d9c9b..0cf759e73f 100644 --- a/weaver/src/main/java/org/aspectj/weaver/tools/cache/SimpleCache.java +++ b/weaver/src/main/java/org/aspectj/weaver/tools/cache/SimpleCache.java @@ -72,6 +72,7 @@ public byte[] getAndInitialize(String classname, byte[] bytes, 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) {