Spring/spring-tools
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Protect AST cache against improper synchronization by using a callback

Browse Source
This commit is contained in:
Kris De Volder
2017-11-20 11:22:56 -08:00
parent 0727f514bf
commit adc4d4f978
4 changed files with 92 additions and 85 deletions
Download Patch File Download Diff File Expand all files Collapse all files

39
headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/annotations/AnnotationHierarchies.java
View File

@@ -36,16 +36,6 @@ import com.google.common.collect.ImmutableList;
*/
public abstract class AnnotationHierarchies {
/**
* Note: this cacehe is not just for efficiency! Without it, we also get into trouble accessing the
* AST on multiple threads. AST's coming from CompilationUnit cache should not be accessed on multiple
* threads.
*/
private static Cache<ITypeBinding, Collection<ITypeBinding>> supertypes = CacheBuilder.newBuilder()
.weakKeys()
.expireAfterWrite(30, TimeUnit.SECONDS)
.build();
private AnnotationHierarchies() {
}
@@ -55,27 +45,20 @@ public abstract class AnnotationHierarchies {
};
public static Collection<ITypeBinding> getDirectSuperAnnotations(ITypeBinding typeBinding) {
try {
return supertypes.get(typeBinding, () -> {
IAnnotationBinding[] annotations = typeBinding.getAnnotations();
if (annotations!=null && annotations.length!=0) {
ImmutableList.Builder<ITypeBinding> superAnnotations = ImmutableList.builder();
for (IAnnotationBinding ab : annotations) {
ITypeBinding sa = ab.getAnnotationType();
if (sa!=null) {
if (!ignoreAnnotation(sa.getQualifiedName())) {
superAnnotations.add(sa);
}
}
IAnnotationBinding[] annotations = typeBinding.getAnnotations();
if (annotations!=null && annotations.length!=0) {
ImmutableList.Builder<ITypeBinding> superAnnotations = ImmutableList.builder();
for (IAnnotationBinding ab : annotations) {
ITypeBinding sa = ab.getAnnotationType();
if (sa!=null) {
if (!ignoreAnnotation(sa.getQualifiedName())) {
superAnnotations.add(sa);
}
return superAnnotations.build();
}
return ImmutableList.of();
});
} catch (ExecutionException e) {
Log.log(e);
return ImmutableList.of();
}
return superAnnotations.build();
}
return ImmutableList.of();
}
public static Set<String> getTransitiveSuperAnnotations(ITypeBinding typeBinding) {

83
headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/handlers/BootJavaHoverProvider.java
View File

@@ -83,51 +83,50 @@ public class BootJavaHoverProvider implements HoverHandler {
}
public Range[] getLiveHoverHints(final TextDocument document, final SpringBootApp[] runningBootApps) {
List<Range> result = new ArrayList<>();
return server.getCompilationUnitCache().withCompilationUnit(document, cu -> {
List<Range> result = new ArrayList<>();
try {
if (cu != null) {
cu.accept(new ASTVisitor() {
@Override
public boolean visit(SingleMemberAnnotation node) {
try {
extractLiveHints(node, document, runningBootApps, result);
} catch (Exception e) {
Log.log(e);
}
try {
CompilationUnit cu = server.getCompilationUnitCache().getCompilationUnit(document);
if (cu != null) {
cu.accept(new ASTVisitor() {
@Override
public boolean visit(SingleMemberAnnotation node) {
try {
extractLiveHints(node, document, runningBootApps, result);
} catch (Exception e) {
Log.log(e);
return super.visit(node);
}
return super.visit(node);
}
@Override
public boolean visit(NormalAnnotation node) {
try {
extractLiveHints(node, document, runningBootApps, result);
} catch (Exception e) {
Log.log(e);
}
@Override
public boolean visit(NormalAnnotation node) {
try {
extractLiveHints(node, document, runningBootApps, result);
} catch (Exception e) {
Log.log(e);
return super.visit(node);
}
return super.visit(node);
}
@Override
public boolean visit(MarkerAnnotation node) {
try {
extractLiveHints(node, document, runningBootApps, result);
} catch (Exception e) {
Log.log(e);
}
@Override
public boolean visit(MarkerAnnotation node) {
try {
extractLiveHints(node, document, runningBootApps, result);
} catch (Exception e) {
Log.log(e);
return super.visit(node);
}
return super.visit(node);
}
});
});
}
} catch (Exception e) {
Log.log(e);
}
} catch (Exception e) {
Log.log(e);
}
return result.toArray(new Range[result.size()]);
return result.toArray(new Range[result.size()]);
});
}
protected void extractLiveHints(Annotation annotation, TextDocument doc, SpringBootApp[] runningApps, List<Range> result) {
@@ -158,11 +157,13 @@ public class BootJavaHoverProvider implements HoverHandler {
private Hover provideHover(TextDocument document, int offset) throws Exception {
IJavaProject project = getProject(document).orElse(null);
if (project!=null) {
CompilationUnit cu = server.getCompilationUnitCache().getCompilationUnit(document);
ASTNode node = NodeFinder.perform(cu, offset, 0);
if (node != null) {
return provideHoverForAnnotation(node, offset, document, project);
}
return server.getCompilationUnitCache().withCompilationUnit(document, cu -> {
ASTNode node = NodeFinder.perform(cu, offset, 0);
if (node != null) {
return provideHoverForAnnotation(node, offset, document, project);
}
return null;
});
}
return null;
}

23
headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/utils/CompilationUnitCache.java
View File

@@ -15,9 +15,11 @@ import java.nio.file.Path;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.locks.ReentrantReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock.ReadLock;
import java.util.concurrent.locks.ReentrantReadWriteLock.WriteLock;
import java.util.function.Function;
import java.util.stream.Stream;
import org.eclipse.jdt.core.JavaCore;
@@ -30,6 +32,7 @@ import org.springframework.ide.vscode.commons.languageserver.java.JavaProjectFin
import org.springframework.ide.vscode.commons.languageserver.java.ProjectObserver;
import org.springframework.ide.vscode.commons.languageserver.util.SimpleTextDocumentService;
import org.springframework.ide.vscode.commons.util.BadLocationException;
import org.springframework.ide.vscode.commons.util.Log;
import org.springframework.ide.vscode.commons.util.text.TextDocument;
import com.google.common.cache.Cache;
@@ -74,14 +77,30 @@ public final class CompilationUnitCache {
}
}
public CompilationUnit getCompilationUnit(TextDocument document) throws Exception {
/**
* Retrieves a CompiationUnitn AST from the cache and passes it to a requestor callback, applying
* proper thred synchronization around the requestor.
* <p>
* Warning: Callers should take care to do all AST processing inside of the requestor callback and
* not pass of AST nodes to helper functions that work aynchronously or store AST nodes or ITypeBindings
* for later use. The JDT ASTs are not thread safe!
*/
public <T> T withCompilationUnit(TextDocument document, Function<CompilationUnit, T> requestor) {
URI uri = URI.create(document.getUri());
readLock.lock();
try {
return uriToCu.get(uri, () -> parse(document));
CompilationUnit cu = uriToCu.get(uri, () -> parse(document));
if (cu!=null) {
synchronized (cu.getAST()) {
return requestor.apply(cu);
}
}
} catch (Exception e) {
Log.log(e);
} finally {
readLock.unlock();
}
return requestor.apply(null);
}
private void invalidateCuForJavaFile(String uriStr) {

32
headless-services/boot-java-language-server/src/test/java/org/springframework/ide/vscode/boot/java/utils/test/CompilationUnitCacheTest.java
View File

@@ -55,13 +55,17 @@ public class CompilationUnitCacheTest {
"public class SomeClass {\n" +
"\n" +
"}\n");
CompilationUnit cu = harness.getServer().getCompilationUnitCache().getCompilationUnit(doc);
CompilationUnit cu = getCompilationUnit(doc);
assertNotNull(cu);
CompilationUnit cuAnother = harness.getServer().getCompilationUnitCache().getCompilationUnit(doc);
CompilationUnit cuAnother = getCompilationUnit(doc);
assertTrue(cu == cuAnother);
}
private CompilationUnit getCompilationUnit(TextDocument doc) {
return harness.getServer().getCompilationUnitCache().withCompilationUnit(doc, cu -> cu);
}
@Test
public void cu_cache_invalidated_by_doc_change() throws Exception {
harness.intialize(null);
@@ -73,15 +77,15 @@ public class CompilationUnitCacheTest {
"}\n");
harness.newEditorFromFileUri(doc.getUri(), doc.getLanguageId());
CompilationUnit cu = harness.getServer().getCompilationUnitCache().getCompilationUnit(doc);
CompilationUnit cu = getCompilationUnit(doc);
assertNotNull(cu);
harness.changeDocument(doc.getUri(), 0, 0, " ");
CompilationUnit cuAnother = harness.getServer().getCompilationUnitCache().getCompilationUnit(doc);
CompilationUnit cuAnother = getCompilationUnit(doc);
assertNotNull(cuAnother);
assertFalse(cu == cuAnother);
CompilationUnit cuYetAnother = harness.getServer().getCompilationUnitCache().getCompilationUnit(doc);
CompilationUnit cuYetAnother = getCompilationUnit(doc);
assertTrue(cuAnother == cuYetAnother);
}
@@ -96,15 +100,15 @@ public class CompilationUnitCacheTest {
"}\n");
harness.newEditorFromFileUri(doc.getUri(), doc.getLanguageId());
CompilationUnit cu = harness.getServer().getCompilationUnitCache().getCompilationUnit(doc);
CompilationUnit cu = getCompilationUnit(doc);
assertNotNull(cu);
harness.closeDocument(doc.getId());
CompilationUnit cuAnother = harness.getServer().getCompilationUnitCache().getCompilationUnit(doc);
CompilationUnit cuAnother = getCompilationUnit(doc);
assertNotNull(cuAnother);
assertFalse(cu == cuAnother);
CompilationUnit cuYetAnother = harness.getServer().getCompilationUnitCache().getCompilationUnit(doc);
CompilationUnit cuYetAnother = getCompilationUnit(doc);
assertTrue(cuAnother == cuYetAnother);
}
@@ -122,13 +126,13 @@ public class CompilationUnitCacheTest {
TextDocument document = new TextDocument(docUri, LanguageId.JAVA, 0, content);
CompilationUnit cu = harness.getServer().getCompilationUnitCache().getCompilationUnit(document);
CompilationUnit cu = getCompilationUnit(document);
assertNotNull(cu);
CompilationUnit cuAnother = harness.getServer().getCompilationUnitCache().getCompilationUnit(document);
CompilationUnit cuAnother = getCompilationUnit(document);
assertTrue(cu == cuAnother);
harness.changeFile(directory.toPath().resolve(MavenCore.POM_XML).toUri().toString());
cuAnother = harness.getServer().getCompilationUnitCache().getCompilationUnit(document);
cuAnother = getCompilationUnit(document);
assertNotNull(cuAnother);
assertFalse(cu == cuAnother);
}
@@ -147,13 +151,13 @@ public class CompilationUnitCacheTest {
TextDocument document = new TextDocument(docUri, LanguageId.JAVA, 0, content);
CompilationUnit cu = harness.getServer().getCompilationUnitCache().getCompilationUnit(document);
CompilationUnit cu = getCompilationUnit(document);
assertNotNull(cu);
CompilationUnit cuAnother = harness.getServer().getCompilationUnitCache().getCompilationUnit(document);
CompilationUnit cuAnother = getCompilationUnit(document);
assertTrue(cu == cuAnother);
harness.deleteFile(directory.toPath().resolve(MavenCore.POM_XML).toUri().toString());
cuAnother = harness.getServer().getCompilationUnitCache().getCompilationUnit(document);
cuAnother = getCompilationUnit(document);
assertNotNull(cuAnother);
assertFalse(cu == cuAnother);
}