mirror of
https://github.com/tiennm99/lombok.git
synced 2026-10-13 14:16:11 +00:00
Fixed issue 164. Oh yes. Party time!
skipPrintAst was a singleton global, so, yes, thread race issues all over the place.
This commit is contained in:
Reinier Zwitserloot committed
1 parent
3be2152a0e
commit
deece27734
6 files changed
+17
-159
No files matched your search
@@ -111,7 +111,7 @@ public class EclipseNode extends lombok.core.LombokNode<EclipseAST, EclipseNode,
|
||||
visitor.visitAnnotationOnLocal((LocalDeclaration)parent.get(), this, (Annotation)get());
|
||||
break;
|
||||
default:
|
||||
throw new AssertionError("Annotion not expected as child of a " + up().getKind());
|
||||
throw new AssertionError("Annotation not expected as child of a " + up().getKind());
|
||||
}
|
||||
break;
|
||||
case STATEMENT:
|
||||
|
||||
@@ -89,8 +89,6 @@ public class HandlerLibrary {
|
||||
|
||||
private Collection<EclipseASTVisitor> visitorHandlers = new ArrayList<EclipseASTVisitor>();
|
||||
|
||||
private boolean skipPrintAST;
|
||||
|
||||
/**
|
||||
* Creates a new HandlerLibrary. Errors will be reported to the Eclipse Error log.
|
||||
* then uses SPI discovery to load all annotation and visitor based handlers so that future calls
|
||||
@@ -170,16 +168,17 @@ public class HandlerLibrary {
|
||||
* @param annotationNode The Lombok AST Node representing the Annotation AST Node.
|
||||
* @param annotation 'node.get()' - convenience parameter.
|
||||
*/
|
||||
public void handleAnnotation(CompilationUnitDeclaration ast, EclipseNode annotationNode, org.eclipse.jdt.internal.compiler.ast.Annotation annotation) {
|
||||
public void handleAnnotation(CompilationUnitDeclaration ast, EclipseNode annotationNode, org.eclipse.jdt.internal.compiler.ast.Annotation annotation, boolean skipPrintAst) {
|
||||
String pkgName = annotationNode.getPackageDeclaration();
|
||||
Collection<String> imports = annotationNode.getImportStatements();
|
||||
|
||||
TypeResolver resolver = new TypeResolver(typeLibrary, pkgName, imports);
|
||||
TypeReference rawType = annotation.type;
|
||||
if (rawType == null) return;
|
||||
|
||||
for (String fqn : resolver.findTypeMatches(annotationNode, toQualifiedName(annotation.type.getTypeName()))) {
|
||||
boolean isPrintAST = fqn.equals(PrintAST.class.getName());
|
||||
if (isPrintAST == skipPrintAST) continue;
|
||||
if (isPrintAST == skipPrintAst) continue;
|
||||
AnnotationHandlerContainer<?> container = annotationHandlers.get(fqn);
|
||||
|
||||
if (container == null) continue;
|
||||
@@ -209,20 +208,4 @@ public class HandlerLibrary {
|
||||
String.format("Lombok visitor handler %s failed", visitor.getClass()), t);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Lombok does not currently support triggering annotations in a specified order; the order is essentially
|
||||
* random right now. This lack of order is particularly annoying for the {@code PrintAST} annotation,
|
||||
* which is almost always intended to run last. Hence, this hack, which lets it in fact run last.
|
||||
*
|
||||
* @see #skipAllButPrintAST()
|
||||
*/
|
||||
public void skipPrintAST() {
|
||||
skipPrintAST = true;
|
||||
}
|
||||
|
||||
/** @see #skipPrintAST() */
|
||||
public void skipAllButPrintAST() {
|
||||
skipPrintAST = false;
|
||||
}
|
||||
}
|
||||
@@ -163,41 +163,45 @@ public class TransformEclipseAST {
|
||||
* then handles any PrintASTs.
|
||||
*/
|
||||
public void go() {
|
||||
handlers.skipPrintAST();
|
||||
ast.traverse(new AnnotationVisitor());
|
||||
ast.traverse(new AnnotationVisitor(true));
|
||||
handlers.callASTVisitors(ast);
|
||||
handlers.skipAllButPrintAST();
|
||||
ast.traverse(new AnnotationVisitor());
|
||||
ast.traverse(new AnnotationVisitor(false));
|
||||
}
|
||||
|
||||
private static class AnnotationVisitor extends EclipseASTAdapter {
|
||||
private final boolean skipPrintAst;
|
||||
|
||||
public AnnotationVisitor(boolean skipAllButPrintAST) {
|
||||
this.skipPrintAst = skipAllButPrintAST;
|
||||
}
|
||||
|
||||
public boolean deferUntilPostDiet() {
|
||||
return false;
|
||||
}
|
||||
|
||||
@Override public void visitAnnotationOnField(FieldDeclaration field, EclipseNode annotationNode, Annotation annotation) {
|
||||
CompilationUnitDeclaration top = (CompilationUnitDeclaration) annotationNode.top().get();
|
||||
handlers.handleAnnotation(top, annotationNode, annotation);
|
||||
handlers.handleAnnotation(top, annotationNode, annotation, skipPrintAst);
|
||||
}
|
||||
|
||||
@Override public void visitAnnotationOnMethodArgument(Argument arg, AbstractMethodDeclaration method, EclipseNode annotationNode, Annotation annotation) {
|
||||
CompilationUnitDeclaration top = (CompilationUnitDeclaration) annotationNode.top().get();
|
||||
handlers.handleAnnotation(top, annotationNode, annotation);
|
||||
handlers.handleAnnotation(top, annotationNode, annotation, skipPrintAst);
|
||||
}
|
||||
|
||||
@Override public void visitAnnotationOnLocal(LocalDeclaration local, EclipseNode annotationNode, Annotation annotation) {
|
||||
CompilationUnitDeclaration top = (CompilationUnitDeclaration) annotationNode.top().get();
|
||||
handlers.handleAnnotation(top, annotationNode, annotation);
|
||||
handlers.handleAnnotation(top, annotationNode, annotation, skipPrintAst);
|
||||
}
|
||||
|
||||
@Override public void visitAnnotationOnMethod(AbstractMethodDeclaration method, EclipseNode annotationNode, Annotation annotation) {
|
||||
CompilationUnitDeclaration top = (CompilationUnitDeclaration) annotationNode.top().get();
|
||||
handlers.handleAnnotation(top, annotationNode, annotation);
|
||||
handlers.handleAnnotation(top, annotationNode, annotation, skipPrintAst);
|
||||
}
|
||||
|
||||
@Override public void visitAnnotationOnType(TypeDeclaration type, EclipseNode annotationNode, Annotation annotation) {
|
||||
CompilationUnitDeclaration top = (CompilationUnitDeclaration) annotationNode.top().get();
|
||||
handlers.handleAnnotation(top, annotationNode, annotation);
|
||||
handlers.handleAnnotation(top, annotationNode, annotation, skipPrintAst);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -25,7 +25,6 @@ import static lombok.eclipse.Eclipse.*;
|
||||
|
||||
import java.lang.reflect.Constructor;
|
||||
import java.lang.reflect.InvocationTargetException;
|
||||
import org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration;
|
||||
import java.lang.reflect.Modifier;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
@@ -38,7 +37,6 @@ import lombok.Getter;
|
||||
import lombok.Lombok;
|
||||
import lombok.core.AnnotationValues;
|
||||
import lombok.core.AST.Kind;
|
||||
import lombok.core.debug.DebugSnapshotStore;
|
||||
import lombok.core.handlers.TransformationsUtil;
|
||||
import lombok.eclipse.Eclipse;
|
||||
import lombok.eclipse.EclipseNode;
|
||||
@@ -451,27 +449,6 @@ public class EclipseHandlerUtil {
|
||||
method.annotations = createSuppressWarningsAll(method, method.annotations);
|
||||
TypeDeclaration parent = (TypeDeclaration) type.get();
|
||||
|
||||
if (parent.scope != null && method.scope == null) {
|
||||
// We think this means heisenbug #164 is about to happen later in some other worker thread.
|
||||
// To improve our ability to figure out what the heck is going on, let's generate a log so we can ask those who stumble on this about it,
|
||||
// and thus see a far more useful stack trace.
|
||||
boolean report = true;
|
||||
for (StackTraceElement elem : Thread.currentThread().getStackTrace()) {
|
||||
// We intentionally hook into the middle of ClassScope filling in BlockScopes for PatchDelegate,
|
||||
// meaning that will trigger a false positive. Detect it and do not report the occurence of #164 if so.
|
||||
if ("lombok.eclipse.agent.PatchDelegate".equals(elem.getClassName())) {
|
||||
report = false;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if (report) {
|
||||
CompilationUnitDeclaration cud = (CompilationUnitDeclaration) type.top().get();
|
||||
String logFileLocation = DebugSnapshotStore.INSTANCE.print(cud, "Printing: injecting whilst scope is already built.");
|
||||
Eclipse.warning("We believe you may have stumbled on issue 164. Please upload file " + logFileLocation + " to: http://code.google.com/p/projectlombok/issues/detail?id=164", new Throwable());
|
||||
}
|
||||
}
|
||||
|
||||
if (parent.methods == null) {
|
||||
parent.methods = new AbstractMethodDeclaration[1];
|
||||
parent.methods[0] = method;
|
||||
|
||||
@@ -21,15 +21,11 @@
|
||||
*/
|
||||
package lombok.eclipse.agent;
|
||||
|
||||
import java.lang.instrument.ClassFileTransformer;
|
||||
import java.lang.instrument.IllegalClassFormatException;
|
||||
import java.lang.instrument.Instrumentation;
|
||||
import java.security.ProtectionDomain;
|
||||
import java.util.Collection;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
|
||||
import lombok.bytecode.ClassFileMetaData;
|
||||
import lombok.core.Agent;
|
||||
import lombok.patcher.Hook;
|
||||
import lombok.patcher.MethodTarget;
|
||||
@@ -72,15 +68,6 @@ public class EclipsePatcher extends Agent {
|
||||
else ecj = injected;
|
||||
|
||||
registerPatchScripts(instrumentation, injected, ecj);
|
||||
instrumentation.addTransformer(new ClassFileTransformer() {
|
||||
@Override public byte[] transform(ClassLoader loader, String className, Class<?> classBeingRedefined, ProtectionDomain protectionDomain, byte[] classfileBuffer) throws IllegalClassFormatException {
|
||||
ClassFileMetaData meta = new ClassFileMetaData(classfileBuffer);
|
||||
if (meta.usesField("org/eclipse/jdt/internal/compiler/ast/TypeDeclaration", "scope")) {
|
||||
return Issue164Fixer.fix(classfileBuffer);
|
||||
}
|
||||
return null;
|
||||
}
|
||||
}, true);
|
||||
}
|
||||
|
||||
private static void registerPatchScripts(Instrumentation instrumentation, boolean reloadExistingClasses, boolean ecjOnly) {
|
||||
|
||||
@@ -1,93 +0,0 @@
|
||||
package lombok.eclipse.agent;
|
||||
|
||||
import lombok.core.debug.DebugSnapshotStore;
|
||||
|
||||
import org.eclipse.jdt.internal.compiler.ast.CompilationUnitDeclaration;
|
||||
import org.eclipse.jdt.internal.compiler.ast.TypeDeclaration;
|
||||
import org.eclipse.jdt.internal.compiler.lookup.ClassScope;
|
||||
import org.eclipse.jdt.internal.compiler.lookup.CompilationUnitScope;
|
||||
import org.eclipse.jdt.internal.compiler.lookup.Scope;
|
||||
import org.objectweb.asm.ClassAdapter;
|
||||
import org.objectweb.asm.ClassReader;
|
||||
import org.objectweb.asm.ClassVisitor;
|
||||
import org.objectweb.asm.ClassWriter;
|
||||
import org.objectweb.asm.MethodAdapter;
|
||||
import org.objectweb.asm.MethodVisitor;
|
||||
import org.objectweb.asm.Opcodes;
|
||||
import org.objectweb.asm.commons.JSRInlinerAdapter;
|
||||
|
||||
public class Issue164Fixer {
|
||||
public static byte[] fix(byte[] classfileBuffer) {
|
||||
return runAsm(classfileBuffer, true);
|
||||
}
|
||||
|
||||
static class FixedClassWriter extends ClassWriter {
|
||||
FixedClassWriter(ClassReader classReader, int flags) {
|
||||
super(classReader, flags);
|
||||
}
|
||||
|
||||
@Override protected String getCommonSuperClass(String type1, String type2) {
|
||||
//By default, ASM will attempt to live-load the class types, which will fail if meddling with classes in an
|
||||
//environment with custom classloaders, such as Equinox. It's just an optimization; returning Object is always legal.
|
||||
try {
|
||||
return super.getCommonSuperClass(type1, type2);
|
||||
} catch (Exception e) {
|
||||
return "java/lang/Object";
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
public static void catchScopeSet(TypeDeclaration typeDeclaration, ClassScope scope) {
|
||||
typeDeclaration.scope = scope;
|
||||
Scope sc = scope;
|
||||
while (sc != null && !(sc instanceof CompilationUnitScope)) {
|
||||
sc = sc.parent;
|
||||
}
|
||||
|
||||
if (sc instanceof CompilationUnitScope) {
|
||||
CompilationUnitDeclaration cud = ((CompilationUnitScope) sc).referenceContext;
|
||||
DebugSnapshotStore.INSTANCE.snapshot(cud, "Scope is being set");
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Runs ASM on the provider byteCode, chaining a reader to a writer and using the {@code ClassVisitor} you yourself provide
|
||||
* via the {@see #createClassVisitor(ClassWriter)} method as the filter.
|
||||
*/
|
||||
protected static byte[] runAsm(byte[] byteCode, boolean computeFrames) {
|
||||
byte[] fixedByteCode = fixJSRInlining(byteCode);
|
||||
|
||||
ClassReader reader = new ClassReader(fixedByteCode);
|
||||
ClassWriter writer = new FixedClassWriter(reader, computeFrames ? ClassWriter.COMPUTE_MAXS | ClassWriter.COMPUTE_FRAMES : 0);
|
||||
|
||||
ClassVisitor visitor = new ClassAdapter(writer) {
|
||||
@Override public MethodVisitor visitMethod(int access, final String mName, String desc, String signature, String[] exceptions) {
|
||||
return new MethodAdapter(super.visitMethod(access, mName, desc, signature, exceptions)) {
|
||||
@Override public void visitFieldInsn(int opcode, String owner, String name, String desc) {
|
||||
if (opcode == Opcodes.PUTFIELD && "org/eclipse/jdt/internal/compiler/ast/TypeDeclaration".equals(owner) && "scope".equals(name) && !"catchScopeSet".equals(mName)) {
|
||||
super.visitMethodInsn(Opcodes.INVOKESTATIC, "lombok/eclipse/agent/Issue164Fixer", "catchScopeSet", "(Lorg/eclipse/jdt/internal/compiler/ast/TypeDeclaration;Lorg/eclipse/jdt/internal/compiler/lookup/ClassScope;)V");
|
||||
} else {
|
||||
super.visitFieldInsn(opcode, owner, name, desc);
|
||||
}
|
||||
}
|
||||
};
|
||||
}
|
||||
};
|
||||
reader.accept(visitor, 0);
|
||||
return writer.toByteArray();
|
||||
}
|
||||
|
||||
protected static byte[] fixJSRInlining(byte[] byteCode) {
|
||||
ClassReader reader = new ClassReader(byteCode);
|
||||
ClassWriter writer = new FixedClassWriter(reader, 0);
|
||||
|
||||
ClassVisitor visitor = new ClassAdapter(writer) {
|
||||
@Override public MethodVisitor visitMethod(int access, String name, String desc, String signature, String[] exceptions) {
|
||||
return new JSRInlinerAdapter(super.visitMethod(access, name, desc, signature, exceptions), access, name, desc, signature, exceptions);
|
||||
}
|
||||
};
|
||||
|
||||
reader.accept(visitor, 0);
|
||||
return writer.toByteArray();
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user