From 9216ef518ac4c9af48c559ee8a6966164ddbb237 Mon Sep 17 00:00:00 2001 From: Andy Clement Date: Tue, 8 Mar 2016 08:52:39 -0800 Subject: [PATCH] Fix 488216: Load-time weaver loses class changes of preceding -javaagent --- .../aspectj/weaver/tools/AbstractTrace.java | 26 ++++-- .../aspectj/weaver/tools/DefaultTrace.java | 93 ++++++++++--------- .../src/org/aspectj/weaver/tools/Trace.java | 56 ++++++----- .../aspectj/weaver/bcel/BcelObjectType.java | 3 + .../org/aspectj/weaver/bcel/BcelWorld.java | 90 +++++++++--------- 5 files changed, 137 insertions(+), 131 deletions(-) diff --git a/org.aspectj.matcher/src/org/aspectj/weaver/tools/AbstractTrace.java b/org.aspectj.matcher/src/org/aspectj/weaver/tools/AbstractTrace.java index 2ec0aa3a9..c736de886 100644 --- a/org.aspectj.matcher/src/org/aspectj/weaver/tools/AbstractTrace.java +++ b/org.aspectj.matcher/src/org/aspectj/weaver/tools/AbstractTrace.java @@ -16,12 +16,15 @@ import java.net.URL; import java.text.SimpleDateFormat; import java.util.Collection; import java.util.Date; +import java.util.regex.Pattern; import org.aspectj.bridge.IMessage.Kind; public abstract class AbstractTrace implements Trace { - protected Class tracedClass; + private static final Pattern packagePrefixPattern = Pattern.compile("([^.])[^.]*(\\.)"); + + protected Class tracedClass; private static SimpleDateFormat timeFormat; @@ -85,13 +88,21 @@ public abstract class AbstractTrace implements Trace { message.append(formatDate(now)).append(" "); message.append(Thread.currentThread().getName()).append(" "); message.append(kind).append(" "); - message.append(className); + message.append(formatClassName(className)); message.append(".").append(methodName); if (thiz != null) message.append(" ").append(formatObj(thiz)); if (args != null) message.append(" ").append(formatArgs(args)); return message.toString(); } + /** + * @param className full dotted class name + * @return short version of class name with package collapse to initials + */ + private String formatClassName(String className) { + return packagePrefixPattern.matcher(className).replaceAll("$1."); + } + protected String formatMessage(String kind, String text, Throwable th) { StringBuffer message = new StringBuffer(); Date now = new Date(); @@ -116,7 +127,7 @@ public abstract class AbstractTrace implements Trace { * NullPointerExceptions or highly verbose results. * * @param obj parameter to be formatted - * @return the formated parameter + * @return the formatted parameter */ protected Object formatObj(Object obj) { @@ -141,14 +152,13 @@ public abstract class AbstractTrace implements Trace { } else try { - /* Classes can provide an alternative implementation of toString() */ + // Classes can provide an alternative implementation of toString() if (obj instanceof Traceable) { - Traceable t = (Traceable)obj; - return t.toTraceString(); + return ((Traceable)obj).toTraceString(); } - /* Use classname@hashcode */ - else return obj.getClass().getName() + "@" + Integer.toHexString(System.identityHashCode(obj)); + // classname@hashcode + else return formatClassName(obj.getClass().getName()) + "@" + Integer.toHexString(System.identityHashCode(obj)); /* Object.hashCode() can be override and may thow an exception */ } catch (Exception ex) { diff --git a/org.aspectj.matcher/src/org/aspectj/weaver/tools/DefaultTrace.java b/org.aspectj.matcher/src/org/aspectj/weaver/tools/DefaultTrace.java index ad95bbb3e..7e2ab0d8f 100644 --- a/org.aspectj.matcher/src/org/aspectj/weaver/tools/DefaultTrace.java +++ b/org.aspectj.matcher/src/org/aspectj/weaver/tools/DefaultTrace.java @@ -12,109 +12,112 @@ package org.aspectj.weaver.tools; import java.io.PrintStream; - public class DefaultTrace extends AbstractTrace { - + private boolean traceEnabled = false; private PrintStream print = System.err; - - public DefaultTrace (Class clazz) { + + public DefaultTrace(Class clazz) { super(clazz); } - - public boolean isTraceEnabled () { + + public boolean isTraceEnabled() { return traceEnabled; } - - public void setTraceEnabled (boolean b) { + + public void setTraceEnabled(boolean b) { traceEnabled = b; } - - public void enter (String methodName, Object thiz, Object[] args) { + + public void enter(String methodName, Object thiz, Object[] args) { if (traceEnabled) { - println(formatMessage(">",tracedClass.getName(),methodName,thiz, args)); + println(formatMessage(">", tracedClass.getName(), methodName, thiz, args)); } } - - public void enter (String methodName, Object thiz) { + + public void enter(String methodName, Object thiz) { if (traceEnabled) { - println(formatMessage(">",tracedClass.getName(),methodName,thiz, null)); + println(formatMessage(">", tracedClass.getName(), methodName, thiz, null)); } } - public void exit (String methodName, Object ret) { + public void exit(String methodName, Object ret) { if (traceEnabled) { - println(formatMessage("<",tracedClass.getName(),methodName,ret, null)); + println(formatMessage("<", tracedClass.getName(), methodName, ret, null)); } } - public void exit (String methodName) { + public void exit(String methodName) { if (traceEnabled) { - println(formatMessage("<",tracedClass.getName(),methodName,null, null)); + println(formatMessage("<", tracedClass.getName(), methodName, null, null)); } } public void exit(String methodName, Throwable th) { if (traceEnabled) { - println(formatMessage("<",tracedClass.getName(),methodName,th, null)); + println(formatMessage("<", tracedClass.getName(), methodName, th, null)); } } - + public void event(String methodName, Object thiz, Object[] args) { if (traceEnabled) { - println(formatMessage("-",tracedClass.getName(),methodName,thiz, args)); + println(formatMessage("-", tracedClass.getName(), methodName, thiz, args)); } } public void event(String methodName) { if (traceEnabled) { - println(formatMessage("-",tracedClass.getName(),methodName,null,null)); + println(formatMessage("-", tracedClass.getName(), methodName, null, null)); } } - public void debug (String message) { - println(formatMessage("?",message,null)); + public void debug(String message) { + println(formatMessage("?", message, null)); } public void info(String message) { - println(formatMessage("I",message,null)); + println(formatMessage("I", message, null)); } - public void warn (String message, Throwable th) { - println(formatMessage("W",message,th)); - if (th != null) th.printStackTrace(); + public void warn(String message, Throwable th) { + println(formatMessage("W", message, th)); + if (th != null) + th.printStackTrace(); } - - public void error (String message, Throwable th) { - println(formatMessage("E",message,th)); - if (th != null) th.printStackTrace(); + public void error(String message, Throwable th) { + println(formatMessage("E", message, th)); + if (th != null) + th.printStackTrace(); } - public void fatal (String message, Throwable th) { - println(formatMessage("X",message,th)); - if (th != null) th.printStackTrace(); + public void fatal(String message, Throwable th) { + println(formatMessage("X", message, th)); + if (th != null) + th.printStackTrace(); } /** * Template method that allows choice of destination for output * - * @param s message to be traced + * @param s + * message to be traced */ - protected void println (String s) { + protected void println(String s) { print.println(s); } - public void setPrintStream (PrintStream printStream) { + public void setPrintStream(PrintStream printStream) { this.print = printStream; } -// private static boolean isTracingEnabled = getBoolean("org.aspectj.weaver.tools.tracing",false); -// -// private static boolean getBoolean (String name, boolean def) { -// String defaultValue = String.valueOf(def); -// String value = System.getProperty(name,defaultValue); -// return Boolean.valueOf(value).booleanValue(); -// } + // private static boolean isTracingEnabled = + // getBoolean("org.aspectj.weaver.tools.tracing",false); + // + // private static boolean getBoolean (String name, boolean def) { + // String defaultValue = String.valueOf(def); + // String value = System.getProperty(name,defaultValue); + // return Boolean.valueOf(value).booleanValue(); + // } } diff --git a/org.aspectj.matcher/src/org/aspectj/weaver/tools/Trace.java b/org.aspectj.matcher/src/org/aspectj/weaver/tools/Trace.java index c1a040d36..8b20032aa 100644 --- a/org.aspectj.matcher/src/org/aspectj/weaver/tools/Trace.java +++ b/org.aspectj.matcher/src/org/aspectj/weaver/tools/Trace.java @@ -12,51 +12,47 @@ package org.aspectj.weaver.tools; public interface Trace { - public void enter (String methodName, Object thiz, Object[] args); + public void enter(String methodName, Object thiz, Object[] args); - public void enter (String methodName, Object thiz); + public void enter(String methodName, Object thiz); - public void exit (String methodName, Object ret); + public void exit(String methodName, Object ret); - public void exit (String methodName, Throwable th); + public void exit(String methodName, Throwable th); - public void exit (String methodName); + public void exit(String methodName); - public void event (String methodName); + public void event(String methodName); - public void event (String methodName, Object thiz, Object[] args); - - public void debug (String message); - - public void info (String message); + public void event(String methodName, Object thiz, Object[] args); - public void warn (String message); + public void debug(String message); - public void warn (String message, Throwable th); + public void info(String message); - public void error (String message); + public void warn(String message); - public void error (String message, Throwable th); + public void warn(String message, Throwable th); - public void fatal (String message); + public void error(String message); - public void fatal (String message, Throwable th); - - - /* - * Convenience methods - */ - public void enter (String methodName, Object thiz, Object arg); + public void error(String message, Throwable th); - public void enter (String methodName, Object thiz, boolean z); + public void fatal(String message); - public void exit (String methodName, boolean b); + public void fatal(String message, Throwable th); - public void exit (String methodName, int i); + public void enter(String methodName, Object thiz, Object arg); - public void event (String methodName, Object thiz, Object arg); - - public boolean isTraceEnabled (); + public void enter(String methodName, Object thiz, boolean z); - public void setTraceEnabled (boolean b); + public void exit(String methodName, boolean b); + + public void exit(String methodName, int i); + + public void event(String methodName, Object thiz, Object arg); + + public boolean isTraceEnabled(); + + public void setTraceEnabled(boolean b); } \ No newline at end of file diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelObjectType.java b/weaver/src/org/aspectj/weaver/bcel/BcelObjectType.java index 584a14c8d..710eb6dc7 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelObjectType.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelObjectType.java @@ -484,6 +484,9 @@ public class BcelObjectType extends AbstractReferenceTypeDelegate { return javaClass; } + /** + * @return true if built from bytes obtained from somewhere. False if built from bytes retrieved from disk. + */ public boolean isArtificial() { return artificial; } diff --git a/weaver/src/org/aspectj/weaver/bcel/BcelWorld.java b/weaver/src/org/aspectj/weaver/bcel/BcelWorld.java index de9e29aba..395e07e10 100644 --- a/weaver/src/org/aspectj/weaver/bcel/BcelWorld.java +++ b/weaver/src/org/aspectj/weaver/bcel/BcelWorld.java @@ -434,12 +434,13 @@ public class BcelWorld extends World implements Repository { if (file == null) { return null; } - ClassParser parser = new ClassParser(file.getInputStream(), file.getPath()); - JavaClass jc = parser.parse(); return jc; } catch (IOException ioe) { + if (trace.isTraceEnabled()) { + trace.error("IOException whilst processing class",ioe); + } return null; } finally { if (file != null) { @@ -448,10 +449,6 @@ public class BcelWorld extends World implements Repository { } } - // public BcelObjectType addSourceObjectType(JavaClass jc) { - // return addSourceObjectType(jc.getClassName(), jc, -1); - // } - public BcelObjectType addSourceObjectType(JavaClass jc, boolean artificial) { return addSourceObjectType(jc.getClassName(), jc, artificial); } @@ -463,19 +460,19 @@ public class BcelWorld extends World implements Repository { } String signature = UnresolvedType.forName(jc.getClassName()).getSignature(); - ResolvedType fromTheMap = typeMap.get(signature); + ResolvedType resolvedTypeFromTypeMap = typeMap.get(signature); - if (fromTheMap != null && !(fromTheMap instanceof ReferenceType)) { + if (resolvedTypeFromTypeMap != null && !(resolvedTypeFromTypeMap instanceof ReferenceType)) { // what on earth is it then? See pr 112243 StringBuffer exceptionText = new StringBuffer(); exceptionText.append("Found invalid (not a ReferenceType) entry in the type map. "); - exceptionText.append("Signature=[" + signature + "] Found=[" + fromTheMap + "] Class=[" + fromTheMap.getClass() + "]"); + exceptionText.append("Signature=[" + signature + "] Found=[" + resolvedTypeFromTypeMap + "] Class=[" + resolvedTypeFromTypeMap.getClass() + "]"); throw new BCException(exceptionText.toString()); } - ReferenceType nameTypeX = (ReferenceType) fromTheMap; + ReferenceType referenceTypeFromTypeMap = (ReferenceType) resolvedTypeFromTypeMap; - if (nameTypeX == null) { + if (referenceTypeFromTypeMap == null) { if (jc.isGeneric() && isInJava5Mode()) { ReferenceType rawType = ReferenceType.fromTypeX(UnresolvedType.forRawTypeName(jc.getClassName()), this); ret = buildBcelDelegate(rawType, jc, artificial, true); @@ -486,67 +483,64 @@ public class BcelWorld extends World implements Repository { rawType.setGenericType(genericRefType); typeMap.put(signature, rawType); } else { - nameTypeX = new ReferenceType(signature, this); - ret = buildBcelDelegate(nameTypeX, jc, artificial, true); - typeMap.put(signature, nameTypeX); + referenceTypeFromTypeMap = new ReferenceType(signature, this); + ret = buildBcelDelegate(referenceTypeFromTypeMap, jc, artificial, true); + typeMap.put(signature, referenceTypeFromTypeMap); } } else { - ret = buildBcelDelegate(nameTypeX, jc, artificial, true); + ret = buildBcelDelegate(referenceTypeFromTypeMap, jc, artificial, true); } return ret; } public BcelObjectType addSourceObjectType(String classname, byte[] bytes, boolean artificial) { - BcelObjectType ret = null; + BcelObjectType retval = null; String signature = UnresolvedType.forName(classname).getSignature(); - ResolvedType fromTheMap = typeMap.get(signature); + ResolvedType resolvedTypeFromTypeMap = typeMap.get(signature); - if (fromTheMap != null && !(fromTheMap instanceof ReferenceType)) { + if (resolvedTypeFromTypeMap != null && !(resolvedTypeFromTypeMap instanceof ReferenceType)) { // what on earth is it then? See pr 112243 StringBuffer exceptionText = new StringBuffer(); exceptionText.append("Found invalid (not a ReferenceType) entry in the type map. "); - exceptionText.append("Signature=[" + signature + "] Found=[" + fromTheMap + "] Class=[" + fromTheMap.getClass() + "]"); + exceptionText.append("Signature=[" + signature + "] Found=[" + resolvedTypeFromTypeMap + "] Class=[" + resolvedTypeFromTypeMap.getClass() + "]"); throw new BCException(exceptionText.toString()); } - ReferenceType nameTypeX = (ReferenceType) fromTheMap; + ReferenceType referenceTypeFromTypeMap = (ReferenceType) resolvedTypeFromTypeMap; - if (nameTypeX == null) { + if (referenceTypeFromTypeMap == null) { JavaClass jc = Utility.makeJavaClass(classname, bytes); if (jc.isGeneric() && isInJava5Mode()) { - nameTypeX = ReferenceType.fromTypeX(UnresolvedType.forRawTypeName(jc.getClassName()), this); - ret = buildBcelDelegate(nameTypeX, jc, artificial, true); + referenceTypeFromTypeMap = ReferenceType.fromTypeX(UnresolvedType.forRawTypeName(jc.getClassName()), this); + retval = buildBcelDelegate(referenceTypeFromTypeMap, jc, artificial, true); ReferenceType genericRefType = new ReferenceType(UnresolvedType.forGenericTypeSignature(signature, - ret.getDeclaredGenericSignature()), this); - nameTypeX.setDelegate(ret); - genericRefType.setDelegate(ret); - nameTypeX.setGenericType(genericRefType); - typeMap.put(signature, nameTypeX); + retval.getDeclaredGenericSignature()), this); + referenceTypeFromTypeMap.setDelegate(retval); + genericRefType.setDelegate(retval); + referenceTypeFromTypeMap.setGenericType(genericRefType); + typeMap.put(signature, referenceTypeFromTypeMap); } else { - nameTypeX = new ReferenceType(signature, this); - ret = buildBcelDelegate(nameTypeX, jc, artificial, true); - typeMap.put(signature, nameTypeX); + referenceTypeFromTypeMap = new ReferenceType(signature, this); + retval = buildBcelDelegate(referenceTypeFromTypeMap, jc, artificial, true); + typeMap.put(signature, referenceTypeFromTypeMap); } } else { - Object o = nameTypeX.getDelegate(); - if (!(o instanceof BcelObjectType)) { - throw new IllegalStateException("For " + classname + " should be BcelObjectType, but is " + o.getClass()); - } - ret = (BcelObjectType) o; - // byte[] bs = ret.javaClass.getBytes(); - // if (bs.length != bytes.length) { - // throw new RuntimeException(""); - // } - // If the type is already exposed to the weaver (ret.isExposedToWeaver()) then this is likely - // to be a hotswap reweave so build a new delegate, dont accidentally use the old data - if (ret.isArtificial() || ret.isExposedToWeaver()) { - // System.out.println("Rebuilding " + nameTypeX.getName()); - ret = buildBcelDelegate(nameTypeX, Utility.makeJavaClass(classname, bytes), artificial, true); - } else { - ret.setExposedToWeaver(true); + ReferenceTypeDelegate existingDelegate = referenceTypeFromTypeMap.getDelegate(); + if (!(existingDelegate instanceof BcelObjectType)) { + throw new IllegalStateException("For " + classname + " should be BcelObjectType, but is " + existingDelegate.getClass()); } + retval = (BcelObjectType) existingDelegate; + // Note1: If the type is already exposed to the weaver (retval.isExposedToWeaver()) then this is likely + // to be a hotswap reweave so build a new delegate, don't accidentally use the old data. + // Note2: Also seen when LTW and another agent precedes the AspectJ agent. Earlier in LTW + // a type is resolved (and ends up in the typemap but not exposed to the weaver at that time) + // then later LTW actually is attempted on this type. We end up here with different + // bytes to the current delegate if the earlier agent has modified them. See PR488216 +// if (retval.isArtificial() || retval.isExposedToWeaver()) { + retval = buildBcelDelegate(referenceTypeFromTypeMap, Utility.makeJavaClass(classname, bytes), artificial, true); +// } } - return ret; + return retval; } void deleteSourceObjectType(UnresolvedType ty) { -- 2.39.5