Skip to content

Commit

Permalink
Fixed issue 164. Oh yes. Party time!
Browse files Browse the repository at this point in the history
skipPrintAst was a singleton global, so, yes, thread race issues all over the place.
  • Loading branch information
rzwitserloot committed Aug 1, 2011
1 parent 3be2152 commit deece27
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 159 deletions.
2 changes: 1 addition & 1 deletion src/core/lombok/eclipse/EclipseNode.java
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public void traverse(EclipseASTVisitor visitor) {
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:
Expand Down
23 changes: 3 additions & 20 deletions src/core/lombok/eclipse/HandlerLibrary.java
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,6 @@ public boolean deferUntilPostDiet() {

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
Expand Down Expand Up @@ -170,16 +168,17 @@ private boolean needsHandling(ASTNode node) {
* @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;
Expand Down Expand Up @@ -209,20 +208,4 @@ public void callASTVisitors(EclipseAST ast) {
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;
}
}
22 changes: 13 additions & 9 deletions src/core/lombok/eclipse/TransformEclipseAST.java
Original file line number Diff line number Diff line change
Expand Up @@ -163,41 +163,45 @@ public TransformEclipseAST(EclipseAST ast) {
* 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);
}
}
}
23 changes: 0 additions & 23 deletions src/core/lombok/eclipse/handlers/EclipseHandlerUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@

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;
Expand All @@ -38,7 +37,6 @@
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;
Expand Down Expand Up @@ -451,27 +449,6 @@ public static void injectMethod(EclipseNode type, AbstractMethodDeclaration meth
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;
Expand Down
13 changes: 0 additions & 13 deletions src/eclipseAgent/lombok/eclipse/agent/EclipsePatcher.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -72,15 +68,6 @@ public void runAgent(String agentArgs, Instrumentation instrumentation, boolean
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) {
Expand Down
93 changes: 0 additions & 93 deletions src/eclipseAgent/lombok/eclipse/agent/Issue164Fixer.java

This file was deleted.

0 comments on commit deece27

Please sign in to comment.