diff --git a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting/InjectedAnnotator.xml b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting/InjectedAnnotator.xml index 2608a489bcec..d7df64fa6900 100644 --- a/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting/InjectedAnnotator.xml +++ b/java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting/InjectedAnnotator.xml @@ -1,5 +1,5 @@ aaa="dddd"> -   +   \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/impl/DaemonRespondToChangesTest.java b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/impl/DaemonRespondToChangesTest.java index c754e56a9dd5..4fcf6add8716 100644 --- a/java/java-tests/testSrc/com/intellij/codeInsight/daemon/impl/DaemonRespondToChangesTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInsight/daemon/impl/DaemonRespondToChangesTest.java @@ -2134,8 +2134,7 @@ public class DaemonRespondToChangesTest extends DaemonAnalyzerTestCase { } public void testAddRemoveHighlighterRaceInIncorrectAnnotatorsWhichUseFileRecursiveVisit() { - Annotator annotator = new MyIncorrectlyRecursiveAnnotator(); - useAnnotatorsIn(new Annotator[]{annotator}, () -> { + useAnnotatorsIn(StdFileTypes.JAVA.getLanguage(), new MyRecordingAnnotator[]{new MyIncorrectlyRecursiveAnnotator()}, () -> { @Language("JAVA") String text1 = "class X {\n" + " int foo(Object param) {\n" + @@ -2158,30 +2157,35 @@ public class DaemonRespondToChangesTest extends DaemonAnalyzerTestCase { }); } - public static void useAnnotatorsIn(@NotNull Annotator[] annotators, @NotNull Runnable runnable) { - com.intellij.lang.Language java = StdFileTypes.JAVA.getLanguage(); + public static void useAnnotatorsIn(@NotNull com.intellij.lang.Language language, + @NotNull MyRecordingAnnotator[] annotators, + @NotNull Runnable runnable) { + MyRecordingAnnotator.clearAll(); for (Annotator annotator : annotators) { - LanguageAnnotators.INSTANCE.addExplicitExtension(java, annotator); + LanguageAnnotators.INSTANCE.addExplicitExtension(language, annotator); } try { - List list = LanguageAnnotators.INSTANCE.allForLanguage(java); + List list = LanguageAnnotators.INSTANCE.allForLanguage(language); assertTrue(list.toString(), list.containsAll(Arrays.asList(annotators))); runnable.run(); + for (MyRecordingAnnotator annotator : annotators) { + assertTrue(annotator +" must have done something but didn't", annotator.didIDoIt()); + } } finally { for (int i = annotators.length - 1; i >= 0; i--) { Annotator annotator = annotators[i]; - LanguageAnnotators.INSTANCE.removeExplicitExtension(java, annotator); + LanguageAnnotators.INSTANCE.removeExplicitExtension(language, annotator); } } - List list = LanguageAnnotators.INSTANCE.allForLanguage(java); + List list = LanguageAnnotators.INSTANCE.allForLanguage(language); for (Annotator annotator : annotators) { assertFalse(list.toString(), list.contains(annotator)); } } - public static class MyIncorrectlyRecursiveAnnotator implements Annotator { + public static class MyIncorrectlyRecursiveAnnotator extends MyRecordingAnnotator { Random random = new Random(); @Override public void annotate(@NotNull PsiElement psiElement, @NotNull AnnotationHolder holder) { @@ -2192,6 +2196,7 @@ public class DaemonRespondToChangesTest extends DaemonAnalyzerTestCase { if (Objects.equals(keyword.getText(), "this")) { holder.createAnnotation(HighlightSeverity.WARNING, keyword.getTextRange(), "XXX"); TimeoutUtil.sleep(random.nextInt(100)); + iDidIt(); } } }); @@ -2383,7 +2388,23 @@ public class DaemonRespondToChangesTest extends DaemonAnalyzerTestCase { } private static final AtomicInteger toSleepMs = new AtomicInteger(0); - public static class MySleepyAnnotator implements Annotator { + public abstract static class MyRecordingAnnotator implements Annotator { + protected static final Set> done = ContainerUtil.newConcurrentSet(); + protected void iDidIt() { + done.add(getClass()); + } + boolean didIDoIt() { + return done.contains(getClass()); + } + static void clearAll() { + done.clear(); + } + } + public static class MySleepyAnnotator extends MyRecordingAnnotator { + public MySleepyAnnotator() { + iDidIt(); // is not supposed to ever do anything + } + @Override public void annotate(@NotNull PsiElement element, @NotNull AnnotationHolder holder) { if (element instanceof PsiClass) { // must be after MyFastAnnotator annotated the comment @@ -2394,33 +2415,39 @@ public class DaemonRespondToChangesTest extends DaemonAnalyzerTestCase { } } } - public static class MyFastAnnotator implements Annotator { + public static class MyFastAnnotator extends MyRecordingAnnotator { private static final String SWEARING = "No swearing"; @Override public void annotate(@NotNull PsiElement element, @NotNull AnnotationHolder holder) { if (element instanceof PsiComment && element.getText().equals("//XXX")) { holder.createErrorAnnotation(element.getTextRange(), SWEARING); + iDidIt(); } } } - public static class MyInfoAnnotator implements Annotator { + public static class MyInfoAnnotator extends MyRecordingAnnotator { @Override public void annotate(@NotNull PsiElement element, @NotNull AnnotationHolder holder) { if (element instanceof PsiComment && ((PsiComment)element).getTokenType().equals(JavaTokenType.C_STYLE_COMMENT)) { holder.createInfoAnnotation(element.getTextRange(), "comment"); + iDidIt(); } } } public void testAddAnnotationToHolderEntailsCreatingCorrespondingRangeHighlighterMoreOrLessImmediately() { if (!ensureEnoughParallelism()) return; - useAnnotatorsIn(new Annotator[]{new MyInfoAnnotator(), new MySleepyAnnotator(), new MyFastAnnotator(), }, this::checkSwearingAnnotationIsVisibleImmediately); - useAnnotatorsIn(new Annotator[]{new MySleepyAnnotator(), new MyInfoAnnotator(), new MyFastAnnotator(), }, this::checkSwearingAnnotationIsVisibleImmediately); - useAnnotatorsIn(new Annotator[]{new MySleepyAnnotator(), new MyFastAnnotator(), new MyInfoAnnotator(), }, this::checkSwearingAnnotationIsVisibleImmediately); + useAnnotatorsIn(StdFileTypes.JAVA.getLanguage(), new MyRecordingAnnotator[]{new MyInfoAnnotator(), new MySleepyAnnotator(), new MyFastAnnotator(), }, this::checkSwearingAnnotationIsVisibleImmediately + ); + useAnnotatorsIn(StdFileTypes.JAVA.getLanguage(), new MyRecordingAnnotator[]{new MySleepyAnnotator(), new MyInfoAnnotator(), new MyFastAnnotator(), }, this::checkSwearingAnnotationIsVisibleImmediately + ); + useAnnotatorsIn(StdFileTypes.JAVA.getLanguage(), new MyRecordingAnnotator[]{new MySleepyAnnotator(), new MyFastAnnotator(), new MyInfoAnnotator(), }, this::checkSwearingAnnotationIsVisibleImmediately + ); // also check in the opposite order in case the order of annotators is important - useAnnotatorsIn(new Annotator[]{new MyFastAnnotator(), new MyInfoAnnotator(), new MySleepyAnnotator(), }, this::checkSwearingAnnotationIsVisibleImmediately); + useAnnotatorsIn(StdFileTypes.JAVA.getLanguage(), new MyRecordingAnnotator[]{new MyFastAnnotator(), new MyInfoAnnotator(), new MySleepyAnnotator(), }, this::checkSwearingAnnotationIsVisibleImmediately + ); } private void checkSwearingAnnotationIsVisibleImmediately() { @@ -2472,7 +2499,7 @@ public class DaemonRespondToChangesTest extends DaemonAnalyzerTestCase { } } - public static class MyNewBuilderAnnotator implements Annotator { + public static class MyNewBuilderAnnotator extends MyRecordingAnnotator { @Override public void annotate(@NotNull PsiElement element, @NotNull AnnotationHolder holder) { if (element instanceof PsiComment && element.getText().equals("//XXX")) { @@ -2483,6 +2510,7 @@ public class DaemonRespondToChangesTest extends DaemonAnalyzerTestCase { while (toSleepMs.addAndGet(-100) > 0) { TimeoutUtil.sleep(100); } + iDidIt(); } } } @@ -2497,7 +2525,8 @@ public class DaemonRespondToChangesTest extends DaemonAnalyzerTestCase { public void testAddAnnotationViaBuilderEntailsCreatingCorrespondingRangeHighlighterImmediately() { if (!ensureEnoughParallelism()) return; - useAnnotatorsIn(new Annotator[]{new MyNewBuilderAnnotator(), }, this::checkSwearingAnnotationIsVisibleImmediately); + useAnnotatorsIn(StdFileTypes.JAVA.getLanguage(), new MyRecordingAnnotator[]{new MyNewBuilderAnnotator(), }, this::checkSwearingAnnotationIsVisibleImmediately + ); } } diff --git a/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/LightAnnotatorHighlightingTest.java b/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/LightAnnotatorHighlightingTest.java index 5e45900e0aa1..8ca9ca4b5dc0 100644 --- a/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/LightAnnotatorHighlightingTest.java +++ b/java/java-tests/testSrc/com/intellij/java/codeInsight/daemon/LightAnnotatorHighlightingTest.java @@ -11,9 +11,10 @@ import com.intellij.codeInsight.daemon.impl.VisibleHighlightingPassFactory; import com.intellij.codeInsight.intention.IntentionAction; import com.intellij.codeInspection.ProblemHighlightType; import com.intellij.codeInspection.deadCode.UnusedDeclarationInspectionBase; -import com.intellij.lang.Language; -import com.intellij.lang.LanguageAnnotators; -import com.intellij.lang.annotation.*; +import com.intellij.lang.annotation.Annotation; +import com.intellij.lang.annotation.AnnotationBuilder; +import com.intellij.lang.annotation.AnnotationHolder; +import com.intellij.lang.annotation.HighlightSeverity; import com.intellij.openapi.editor.Editor; import com.intellij.openapi.editor.colors.CodeInsightColors; import com.intellij.openapi.editor.ex.EditorEx; @@ -44,29 +45,13 @@ import java.util.function.Function; public class LightAnnotatorHighlightingTest extends LightDaemonAnalyzerTestCase { public void testInjectedAnnotator() { - Annotator annotator = new MyAnnotator(); - Language xml = StdFileTypes.XML.getLanguage(); - LanguageAnnotators.INSTANCE.addExplicitExtension(xml, annotator); - try { - List list = LanguageAnnotators.INSTANCE.allForLanguage(xml); - assertTrue(list.toString(), list.contains(annotator)); + DaemonRespondToChangesTest.useAnnotatorsIn(StdFileTypes.XML.getLanguage(), new DaemonRespondToChangesTest.MyRecordingAnnotator[]{new MyAnnotator()}, () -> { doTest(LightAdvHighlightingTest.BASE_PATH + "/" + getTestName(false) + ".xml",true,false); - } - finally { - LanguageAnnotators.INSTANCE.removeExplicitExtension(xml, annotator); - } - - List list = LanguageAnnotators.INSTANCE.allForLanguage(xml); - assertFalse(list.toString(), list.contains(annotator)); + }); } public void testAnnotatorWorksWithFileLevel() { - Annotator annotator = new MyTopFileAnnotator(); - Language java = StdFileTypes.JAVA.getLanguage(); - LanguageAnnotators.INSTANCE.addExplicitExtension(java, annotator); - try { - List list = LanguageAnnotators.INSTANCE.allForLanguage(java); - assertTrue(list.toString(), list.contains(annotator)); + DaemonRespondToChangesTest.useAnnotatorsIn(StdFileTypes.JAVA.getLanguage(), new DaemonRespondToChangesTest.MyRecordingAnnotator[]{new MyTopFileAnnotator()}, () -> { configureByFile(LightAdvHighlightingTest.BASE_PATH + "/" + getTestName(false) + ".java"); ((EditorEx)getEditor()).getScrollPane().getViewport().setSize(new Dimension(1000,1000)); // whole file fit onscreen doHighlighting(); @@ -87,30 +72,28 @@ public class LightAnnotatorHighlightingTest extends LightDaemonAnalyzerTestCase assertEmpty(warnings); fileLevel = ((DaemonCodeAnalyzerImpl)DaemonCodeAnalyzer.getInstance(getProject())).getFileLevelHighlights(getProject(), getFile()); assertEmpty(fileLevel); - } - finally { - LanguageAnnotators.INSTANCE.removeExplicitExtension(java, annotator); - } - - List list = LanguageAnnotators.INSTANCE.allForLanguage(java); - assertFalse(list.toString(), list.contains(annotator)); + }); } // must stay public for PicoContainer to work - public static class MyAnnotator implements Annotator { + public static final class MyAnnotator extends DaemonRespondToChangesTest.MyRecordingAnnotator { @Override public void annotate(@NotNull PsiElement psiElement, @NotNull AnnotationHolder holder) { psiElement.accept(new XmlElementVisitor() { - @Override public void visitXmlTag(XmlTag tag) { + @Override + public void visitXmlTag(XmlTag tag) { XmlAttribute attribute = tag.getAttribute("aaa", ""); if (attribute != null) { holder.createWarningAnnotation(attribute, "MyAnnotator"); + iDidIt(); } } - @Override public void visitXmlToken(XmlToken token) { - if (token.getTokenType() == XmlTokenType.XML_ENTITY_REF_TOKEN) { + @Override + public void visitXmlToken(XmlToken token) { + if (token.getTokenType() == XmlTokenType.XML_CHAR_ENTITY_REF) { holder.createWarningAnnotation(token, "ENTITY"); + iDidIt(); } } }); @@ -118,18 +101,20 @@ public class LightAnnotatorHighlightingTest extends LightDaemonAnalyzerTestCase } // must stay public for PicoContainer to work - public static class MyTopFileAnnotator implements Annotator { + public static class MyTopFileAnnotator extends DaemonRespondToChangesTest.MyRecordingAnnotator { @Override public void annotate(@NotNull PsiElement psiElement, @NotNull AnnotationHolder holder) { if (psiElement instanceof PsiFile && !psiElement.getText().contains("xxx")) { Annotation annotation = holder.createWarningAnnotation(psiElement, "top level"); annotation.setFileLevelAnnotation(true); + iDidIt(); } } } public void testAnnotatorMustNotSpecifyCrazyRangeForCreatedAnnotation() { - DaemonRespondToChangesTest.useAnnotatorsIn(new Annotator[]{new MyCrazyAnnotator()}, () -> runMyAnnotators()); + DaemonRespondToChangesTest.useAnnotatorsIn(StdFileTypes.JAVA.getLanguage(), new DaemonRespondToChangesTest.MyRecordingAnnotator[]{new MyCrazyAnnotator()}, () -> runMyAnnotators() + ); } private void runMyAnnotators() { @org.intellij.lang.annotations.Language("JAVA") @@ -147,11 +132,12 @@ public class LightAnnotatorHighlightingTest extends LightDaemonAnalyzerTestCase .runPasses(getFile(), getEditor().getDocument(), Collections.singletonList(textEditor), ArrayUtilRt.EMPTY_INT_ARRAY, false, null); } - public static class MyCrazyAnnotator implements Annotator { + public static class MyCrazyAnnotator extends DaemonRespondToChangesTest.MyRecordingAnnotator { @Override public void annotate(@NotNull PsiElement element, @NotNull AnnotationHolder holder) { if (element instanceof PsiComment && element.getText().equals("//XXX")) { try { + iDidIt(); holder.newAnnotation(HighlightSeverity.ERROR, "xxx") .range(new TextRange(0,1)) .create(); @@ -224,7 +210,7 @@ public class LightAnnotatorHighlightingTest extends LightDaemonAnalyzerTestCase assertSame(newBuilder, builder); builder.registerFix().create(); } - public static class MyStupidRepetitiveAnnotator implements Annotator { + public static class MyStupidRepetitiveAnnotator extends DaemonRespondToChangesTest.MyRecordingAnnotator { @Override public void annotate(@NotNull PsiElement element, @NotNull AnnotationHolder holder) { if (element instanceof PsiComment && element.getText().equals("//XXX")) { @@ -258,11 +244,13 @@ public class LightAnnotatorHighlightingTest extends LightDaemonAnalyzerTestCase HighlightDisplayKey myDeadCodeKey = HighlightDisplayKey.findOrRegister(UnusedDeclarationInspectionBase.SHORT_NAME, UnusedDeclarationInspectionBase.DISPLAY_NAME, UnusedDeclarationInspectionBase.SHORT_NAME); checkThrowsWhenCalledTwiceOnFixBuilder(holder, fixBuilder -> fixBuilder.key(myDeadCodeKey)); checkThrowsWhenCalledTwiceOnFixBuilder(holder, fixBuilder -> fixBuilder.range(new TextRange(0,0))); + iDidIt(); } } } public void testAnnotationBuilderMethodsAllowedToBeCalledOnlyOnce() { - DaemonRespondToChangesTest.useAnnotatorsIn(new Annotator[]{new MyStupidRepetitiveAnnotator()}, () -> runMyAnnotators()); + DaemonRespondToChangesTest.useAnnotatorsIn(StdFileTypes.JAVA.getLanguage(), new DaemonRespondToChangesTest.MyRecordingAnnotator[]{new MyStupidRepetitiveAnnotator()}, () -> runMyAnnotators() + ); } } \ No newline at end of file diff --git a/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/DaemonCodeAnalyzerImpl.java b/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/DaemonCodeAnalyzerImpl.java index 4d4a5abbee46..273eeb311551 100644 --- a/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/DaemonCodeAnalyzerImpl.java +++ b/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/DaemonCodeAnalyzerImpl.java @@ -323,7 +323,8 @@ public final class DaemonCodeAnalyzerImpl extends DaemonCodeAnalyzerEx implement } myUpdateRunnableFuture.cancel(false); - waitForTermination(); // previous passes can be canceled but still in flight. wait for them to avoid interference + // previous passes can be canceled but still in flight. wait for them to avoid interference + myPassExecutorService.cancelAll(false); fileStatusMap.allowDirt(canChangeDocument); final DaemonProgressIndicator progress = createUpdateProgress(map.keySet()); myPassExecutorService.submitPasses(map, progress); @@ -350,7 +351,7 @@ public final class DaemonCodeAnalyzerImpl extends DaemonCodeAnalyzerEx implement HighlightingSessionImpl session = (HighlightingSessionImpl)HighlightingSessionImpl.getOrCreateHighlightingSession(file, progress, null); if (!waitInOtherThread(60000, canChangeDocument)) { - throw new TimeoutException("Unable to complete in 60s"); + throw new TimeoutException("Unable to complete in 60s. Thread dump:\n"+ThreadDumper.dumpThreadsToString()); } session.waitForHighlightInfosApplied(); UIUtil.dispatchAllInvocationEvents(); diff --git a/platform/platform-impl/src/com/intellij/concurrency/JobLauncherImpl.java b/platform/platform-impl/src/com/intellij/concurrency/JobLauncherImpl.java index 8b16d43e9765..9ee1923aa010 100644 --- a/platform/platform-impl/src/com/intellij/concurrency/JobLauncherImpl.java +++ b/platform/platform-impl/src/com/intellij/concurrency/JobLauncherImpl.java @@ -3,7 +3,6 @@ package com.intellij.concurrency; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.application.ex.ApplicationUtil; -import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.progress.ProcessCanceledException; import com.intellij.openapi.progress.ProgressIndicator; import com.intellij.openapi.progress.ProgressManager; @@ -26,7 +25,6 @@ import java.util.concurrent.atomic.AtomicBoolean; * @author cdr */ public class JobLauncherImpl extends JobLauncher { - private static final Logger LOG = Logger.getInstance(JobLauncherImpl.class); static final int CORES_FORK_THRESHOLD = 1; @Override @@ -215,23 +213,26 @@ public class JobLauncherImpl extends JobLauncher { // waits for the job to finish execution (when called on a canceled job in the middle of the execution, wait for finish) @Override - public void waitForCompletion(int millis) throws InterruptedException, ExecutionException, TimeoutException { + public void waitForCompletion(int millis) throws InterruptedException, TimeoutException { + long timeout = System.currentTimeMillis() + millis; while (!isDone()) { + long toWait = timeout - System.currentTimeMillis(); + if (toWait < 0) { + throw new TimeoutException(); + } try { - myForkJoinTask.get(millis, TimeUnit.MILLISECONDS); - break; + myForkJoinTask.get(toWait, TimeUnit.MILLISECONDS); } catch (CancellationException e) { // was canceled in the middle of execution - // can't do anything but wait. help other tasks in the meantime - if (!isDone()) { - ForkJoinPool.commonPool().awaitQuiescence(millis, TimeUnit.MILLISECONDS); - if (!isDone()) throw new TimeoutException(); - } } catch (ExecutionException e) { ExceptionUtil.rethrow(e.getCause()); } + // can't do anything but wait. help other tasks in the meantime + if (!isDone()) { + ForkJoinPool.commonPool().awaitQuiescence(toWait, TimeUnit.MILLISECONDS); + } } } }