From 1536b4f8fe85d81d83c8d91e72687fd436abd1ce Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Tue, 23 Jul 2024 10:17:10 +0200 Subject: [PATCH] do not rely on ExternalAnnotatorManager.wait(), wait for this particular task instead to fix IJPL-28996 Lock in ExternalAnnotatorManager.waitForAllExecuted GitOrigin-RevId: 71a742c28d352e58e79b761195991412a60498ef --- .../daemon/impl/ExternalToolPass.java | 77 +++++++++++-------- 1 file changed, 47 insertions(+), 30 deletions(-) diff --git a/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/ExternalToolPass.java b/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/ExternalToolPass.java index 523c19a0e450..b27b8918abb8 100644 --- a/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/ExternalToolPass.java +++ b/platform/lang-impl/src/com/intellij/codeInsight/daemon/impl/ExternalToolPass.java @@ -26,20 +26,21 @@ import com.intellij.openapi.progress.ProgressManager; import com.intellij.openapi.progress.util.BackgroundTaskUtil; import com.intellij.openapi.project.DumbAware; import com.intellij.openapi.project.DumbService; +import com.intellij.openapi.util.ProperTextRange; import com.intellij.openapi.util.TextRange; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.profile.codeInspection.InspectionProjectProfileManager; import com.intellij.psi.FileViewProvider; import com.intellij.psi.PsiFile; +import com.intellij.util.TimeoutUtil; +import com.intellij.util.concurrency.annotations.RequiresBackgroundThread; import com.intellij.util.containers.ContainerUtil; import com.intellij.util.ui.update.Update; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.*; -import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; import java.util.function.Function; public final class ExternalToolPass extends ProgressableTextEditorHighlightingPass implements DumbAware { @@ -47,6 +48,7 @@ public final class ExternalToolPass extends ProgressableTextEditorHighlightingPa private final List> myAnnotationData = Collections.synchronizedList(new ArrayList<>()); private volatile @NotNull List myHighlightInfos = Collections.emptyList(); + private volatile boolean externalUpdateTaskCompleted; // true when the task in ExternalAnnotatorManager.getInstance().queue() was completed private static final class MyData { final @NotNull ExternalAnnotator annotator; @@ -145,31 +147,41 @@ public final class ExternalToolPass extends ProgressableTextEditorHighlightingPa ExternalAnnotatorManager.getInstance().queue(new Update(myFile) { @Override public void setRejected() { - super.setRejected(); - if (!myProject.isDisposed()) { // Project close in EDT might call MergeUpdateQueue.dispose which calls setRejected in EDT - doFinish(); + try { + super.setRejected(); + if (!myProject.isDisposed()) { // Project close in EDT might call MergeUpdateQueue.dispose which calls setRejected in EDT + doFinish(); + } + } + finally { + externalUpdateTaskCompleted = true; } } @Override public void run() { - if (documentChanged(modificationStampBefore) || myProject.isDisposed()) { - return; - } + try { + if (documentChanged(modificationStampBefore) || myProject.isDisposed()) { + return; + } - // have to instantiate new indicator because the old one (progress) might have already been canceled - DaemonProgressIndicator indicator = new DaemonProgressIndicator(); - BackgroundTaskUtil.runUnderDisposeAwareIndicator(myProject, () -> { - // run annotators outside the read action because they could start OSProcessHandler - runChangeAware(myDocument, () -> doAnnotate()); - ReadAction.run(() -> { - ProgressManager.checkCanceled(); - if (!documentChanged(modificationStampBefore)) { - doApply(); - doFinish(); - } - }); - }, indicator); + // have to instantiate new indicator because the old one (progress) might have already been canceled + DaemonProgressIndicator indicator = new DaemonProgressIndicator(); + BackgroundTaskUtil.runUnderDisposeAwareIndicator(myProject, () -> { + // run annotators outside the read action because they could start OSProcessHandler + runChangeAware(myDocument, () -> doAnnotate()); + ReadAction.run(() -> { + ProgressManager.checkCanceled(); + if (!documentChanged(modificationStampBefore)) { + doApply(); + doFinish(); + } + }); + }, indicator); + } + finally { + externalUpdateTaskCompleted = true; + } } }); } @@ -177,11 +189,13 @@ public final class ExternalToolPass extends ProgressableTextEditorHighlightingPa @Override public @NotNull List getInfos() { ApplicationManager.getApplication().assertIsNonDispatchThread(); - try { - ExternalAnnotatorManager.getInstance().waitForAllExecuted(1, TimeUnit.MINUTES); - } - catch (ExecutionException | InterruptedException | TimeoutException e) { - throw new RuntimeException(e); + long deadline = System.currentTimeMillis() + TimeUnit.SECONDS.toMillis(60); + + while (!externalUpdateTaskCompleted) { + ProgressManager.checkCanceled(); + TimeoutUtil.sleep(1); + Thread.yield(); + if (System.currentTimeMillis() > deadline) break; } //noinspection unchecked return (List)myHighlightInfos; @@ -231,16 +245,19 @@ public final class ExternalToolPass extends ProgressableTextEditorHighlightingPa } } + @RequiresBackgroundThread private void doFinish() { List highlights = myAnnotationData.stream() .flatMap(data -> ContainerUtil.notNullize(data.annotationHolder).stream().map(annotation -> HighlightInfo.fromAnnotation(data.annotator, annotation))) .toList(); MarkupModelEx markupModel = (MarkupModelEx)DocumentMarkupModel.forDocument(myDocument, myProject, true); - // use the method which doesn't retrieve a HighlightingSession from the indicator, because we likely destroyed the one already - BackgroundUpdateHighlightersUtil.setHighlightersInRange(myRestrictRange, highlights, markupModel, getId(), getHighlightingSession()); - DaemonCodeAnalyzerEx.getInstanceEx(myProject).getFileStatusMap().markFileUpToDate(myDocument, getId()); - myHighlightInfos = highlights; + HighlightingSessionImpl.runInsideHighlightingSession(myFile, getColorsScheme(), ProperTextRange.create(myFile.getTextRange()), false, session -> { + // use the method which doesn't retrieve a HighlightingSession from the indicator, because we likely destroyed the one already + BackgroundUpdateHighlightersUtil.setHighlightersInRange(myRestrictRange, highlights, markupModel, getId(), session); + DaemonCodeAnalyzerEx.getInstanceEx(myProject).getFileStatusMap().markFileUpToDate(myDocument, getId()); + myHighlightInfos = highlights; + }); } private static void processError(@NotNull Throwable t, @NotNull ExternalAnnotator annotator, @NotNull PsiFile root) {