diff --git a/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/annotations/AnnotationHierarchies.java b/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/annotations/AnnotationHierarchies.java index 7feb9a8c7..f5f480738 100644 --- a/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/annotations/AnnotationHierarchies.java +++ b/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/annotations/AnnotationHierarchies.java @@ -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> supertypes = CacheBuilder.newBuilder() - .weakKeys() - .expireAfterWrite(30, TimeUnit.SECONDS) - .build(); - private AnnotationHierarchies() { } @@ -55,27 +45,20 @@ public abstract class AnnotationHierarchies { }; public static Collection getDirectSuperAnnotations(ITypeBinding typeBinding) { - try { - return supertypes.get(typeBinding, () -> { - IAnnotationBinding[] annotations = typeBinding.getAnnotations(); - if (annotations!=null && annotations.length!=0) { - ImmutableList.Builder 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 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 getTransitiveSuperAnnotations(ITypeBinding typeBinding) { diff --git a/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/handlers/BootJavaHoverProvider.java b/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/handlers/BootJavaHoverProvider.java index d22b0fbca..f169e589f 100644 --- a/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/handlers/BootJavaHoverProvider.java +++ b/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/handlers/BootJavaHoverProvider.java @@ -83,51 +83,50 @@ public class BootJavaHoverProvider implements HoverHandler { } public Range[] getLiveHoverHints(final TextDocument document, final SpringBootApp[] runningBootApps) { - List result = new ArrayList<>(); + return server.getCompilationUnitCache().withCompilationUnit(document, cu -> { + List 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 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; } diff --git a/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/utils/CompilationUnitCache.java b/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/utils/CompilationUnitCache.java index 776c638d1..a3111cbc6 100644 --- a/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/utils/CompilationUnitCache.java +++ b/headless-services/boot-java-language-server/src/main/java/org/springframework/ide/vscode/boot/java/utils/CompilationUnitCache.java @@ -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. + *

+ * 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 withCompilationUnit(TextDocument document, Function 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) { diff --git a/headless-services/boot-java-language-server/src/test/java/org/springframework/ide/vscode/boot/java/utils/test/CompilationUnitCacheTest.java b/headless-services/boot-java-language-server/src/test/java/org/springframework/ide/vscode/boot/java/utils/test/CompilationUnitCacheTest.java index db3f0fbfa..367cfe26f 100644 --- a/headless-services/boot-java-language-server/src/test/java/org/springframework/ide/vscode/boot/java/utils/test/CompilationUnitCacheTest.java +++ b/headless-services/boot-java-language-server/src/test/java/org/springframework/ide/vscode/boot/java/utils/test/CompilationUnitCacheTest.java @@ -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); }