From 43f45f27de54530b471e05dbfe5e66919e8daac7 Mon Sep 17 00:00:00 2001 From: azerr Date: Thu, 10 Aug 2023 17:46:19 +0200 Subject: [PATCH] fix: Too many non-blocking read actions submitted at once in LSPDiagnosticHandler Fixes #1089 Signed-off-by: azerr --- .../client/IndexAwareLanguageClient.java | 14 ++++++++++++- .../lsp4ij/client/LSPCompletableFuture.java | 8 ++++++- .../diagnostics/LSPDiagnosticHandler.java | 16 +++++++++++--- .../ClasspathResourceChangedNotifier.java | 21 ++++++++++++++----- .../jaxrs/java/DefaultJaxRsInfoProvider.java | 5 +++++ .../quarkus/lsp/QuarkusLanguageClient.java | 4 +++- .../intellij/qute/lsp/QuteLanguageClient.java | 5 +++-- 7 files changed, 60 insertions(+), 13 deletions(-) diff --git a/src/main/java/com/redhat/devtools/intellij/lsp4ij/client/IndexAwareLanguageClient.java b/src/main/java/com/redhat/devtools/intellij/lsp4ij/client/IndexAwareLanguageClient.java index 7b77b1ffa..ce239c687 100644 --- a/src/main/java/com/redhat/devtools/intellij/lsp4ij/client/IndexAwareLanguageClient.java +++ b/src/main/java/com/redhat/devtools/intellij/lsp4ij/client/IndexAwareLanguageClient.java @@ -39,7 +39,19 @@ public IndexAwareLanguageClient(Project project) { * @return the output of the function code */ protected CompletableFuture runAsBackground(String progressTitle, Function code) { - return new LSPCompletableFuture<>(code, progressTitle, IndexAwareLanguageClient.this); + return runAsBackground(progressTitle, code, null); + } + + /** + * Run the given function as a background task, wrapped in a read action + * + * @param progressTitle the progress title of the action being run + * @param code the function code to execute in the background + * @param the return type + * @return the output of the function code + */ + protected CompletableFuture runAsBackground(String progressTitle, Function code, Object coalesceBy) { + return new LSPCompletableFuture<>(code, progressTitle, IndexAwareLanguageClient.this, coalesceBy); } /** diff --git a/src/main/java/com/redhat/devtools/intellij/lsp4ij/client/LSPCompletableFuture.java b/src/main/java/com/redhat/devtools/intellij/lsp4ij/client/LSPCompletableFuture.java index 9b7141d3a..ca63cb868 100644 --- a/src/main/java/com/redhat/devtools/intellij/lsp4ij/client/LSPCompletableFuture.java +++ b/src/main/java/com/redhat/devtools/intellij/lsp4ij/client/LSPCompletableFuture.java @@ -54,12 +54,15 @@ public ResultOrError(R result, Exception error) { private final IndexAwareLanguageClient languageClient; private final String progressTitle; private final AtomicInteger nbAttempt; + + private final Object coalesceBy; private CancellablePromise> nonBlockingReadActionPromise; - public LSPCompletableFuture(Function code, String progressTitle, IndexAwareLanguageClient languageClient) { + public LSPCompletableFuture(Function code, String progressTitle, IndexAwareLanguageClient languageClient, Object coalesceBy) { this.code = code; this.progressTitle = progressTitle; this.languageClient = languageClient; + this.coalesceBy = coalesceBy; this.nbAttempt = new AtomicInteger(0); // if indexation is processing, we need to execute the promise in smart mode var executeInSmartMode = DumbService.getInstance(languageClient.getProject()).isDumb(); @@ -136,6 +139,9 @@ private CancellablePromise> nonBlockingReadActionPromise(boolea if (executeInSmartMode) { action = action.inSmartMode(project); } + if (coalesceBy != null) { + action = action.coalesceBy(coalesceBy); + } return action .submit(AppExecutorUtil.getAppExecutorService()); } diff --git a/src/main/java/com/redhat/devtools/intellij/lsp4ij/operations/diagnostics/LSPDiagnosticHandler.java b/src/main/java/com/redhat/devtools/intellij/lsp4ij/operations/diagnostics/LSPDiagnosticHandler.java index 67f271dfc..720023465 100644 --- a/src/main/java/com/redhat/devtools/intellij/lsp4ij/operations/diagnostics/LSPDiagnosticHandler.java +++ b/src/main/java/com/redhat/devtools/intellij/lsp4ij/operations/diagnostics/LSPDiagnosticHandler.java @@ -17,6 +17,7 @@ import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.application.ReadAction; import com.intellij.openapi.module.Module; +import com.intellij.openapi.project.DumbService; import com.intellij.openapi.project.Project; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.psi.PsiFile; @@ -45,12 +46,21 @@ public LSPDiagnosticHandler(LanguageServerWrapper languageServerWrapper) { @Override public void accept(PublishDiagnosticsParams params) { + Project project = languageServerWrapper.getProject(); + if(project.isDisposed()) { + return; + } if (ApplicationManager.getApplication().isReadAccessAllowed()) { updateDiagnostics(params); } else { - ReadAction.nonBlocking(() -> updateDiagnostics(params)) - .submit(AppExecutorUtil.getAppExecutorService()); - + var executeInSmartMode = DumbService.getInstance(languageServerWrapper.getProject()).isDumb(); + var action = ReadAction.nonBlocking(() -> updateDiagnostics(params)) + .expireWith(languageServerWrapper) + .coalesceBy(params); + if (executeInSmartMode) { + action.inSmartMode(project); + } + action.submit(AppExecutorUtil.getAppExecutorService()); } } diff --git a/src/main/java/com/redhat/devtools/intellij/lsp4mp4ij/classpath/ClasspathResourceChangedNotifier.java b/src/main/java/com/redhat/devtools/intellij/lsp4mp4ij/classpath/ClasspathResourceChangedNotifier.java index 35fb93c36..f5f0e1608 100644 --- a/src/main/java/com/redhat/devtools/intellij/lsp4mp4ij/classpath/ClasspathResourceChangedNotifier.java +++ b/src/main/java/com/redhat/devtools/intellij/lsp4mp4ij/classpath/ClasspathResourceChangedNotifier.java @@ -35,7 +35,7 @@ /** * Source file change notifier with a debounce mode. */ -public class ClasspathResourceChangedNotifier implements Disposable { +public class ClasspathResourceChangedNotifier implements Disposable { private static final long DEBOUNCE_DELAY = 1000; @@ -46,7 +46,8 @@ public class ClasspathResourceChangedNotifier implements Disposable { private final Set> sourceFiles; private boolean librariesChanged; -private final List processBeforeLibrariesChanged; + private final List processBeforeLibrariesChanged; + private boolean disposed; public ClasspathResourceChangedNotifier(Project project, List preprocessors) { this.project = project; @@ -74,6 +75,9 @@ public synchronized void addSourceFile(Pair pair) { } private void asyncNotifyChanges() { + if (isDisposed()) { + return; + } if (ApplicationManager.getApplication().isUnitTestMode()) { notifyChanges(); } else { @@ -93,6 +97,9 @@ public void run() { } private void notifyChanges() { + if (isDisposed()) { + return; + } synchronized (sourceFiles) { // Java, config sources files has changed project.getMessageBus().syncPublisher(ClasspathResourceChangedManager.TOPIC).sourceFilesChanged(sourceFiles); @@ -121,8 +128,7 @@ public void run(@NotNull ProgressIndicator progressIndicator) { for (var runnable : processBeforeLibrariesChanged) { runnable.run(progressIndicator); } - } - finally { + } finally { // Send the libraries changed event project.getMessageBus().syncPublisher(ClasspathResourceChangedManager.TOPIC).librariesChanged(); librariesChanged = false; @@ -134,12 +140,17 @@ public void run(@NotNull ProgressIndicator progressIndicator) { } } + public boolean isDisposed() { + return disposed; + } + @Override public void dispose() { + this.disposed = true; if (debounceTask != null) { debounceTask.cancel(); } - if(debounceTimer != null) { + if (debounceTimer != null) { debounceTimer.cancel(); } } diff --git a/src/main/java/com/redhat/devtools/intellij/lsp4mp4ij/psi/internal/jaxrs/java/DefaultJaxRsInfoProvider.java b/src/main/java/com/redhat/devtools/intellij/lsp4mp4ij/psi/internal/jaxrs/java/DefaultJaxRsInfoProvider.java index 94f24433d..3eabafcda 100644 --- a/src/main/java/com/redhat/devtools/intellij/lsp4mp4ij/psi/internal/jaxrs/java/DefaultJaxRsInfoProvider.java +++ b/src/main/java/com/redhat/devtools/intellij/lsp4mp4ij/psi/internal/jaxrs/java/DefaultJaxRsInfoProvider.java @@ -14,7 +14,9 @@ package com.redhat.devtools.intellij.lsp4mp4ij.psi.internal.jaxrs.java; import com.intellij.openapi.module.Module; +import com.intellij.openapi.progress.ProcessCanceledException; import com.intellij.openapi.progress.ProgressIndicator; +import com.intellij.openapi.project.IndexNotReadyException; import com.intellij.psi.*; import com.intellij.psi.util.PsiTreeUtil; import com.redhat.devtools.intellij.lsp4ij.LSPIJUtils; @@ -28,6 +30,7 @@ import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.concurrent.CancellationException; import java.util.logging.Level; import java.util.logging.Logger; @@ -64,6 +67,8 @@ public List getJaxRsMethodInfo(PsiFile typeRoot, JaxRsContext j List methodInfos = new ArrayList<>(); try { collectJaxRsMethodInfo(typeRoot.getChildren(), null, methodInfos, jaxrsContext, utils, monitor); + } catch (IndexNotReadyException | ProcessCanceledException | CancellationException e) { + throw e; } catch (Exception e) { LOGGER.log(Level.SEVERE, "while collecting JAX-RS method info using the default method", e); } diff --git a/src/main/java/com/redhat/devtools/intellij/quarkus/lsp/QuarkusLanguageClient.java b/src/main/java/com/redhat/devtools/intellij/quarkus/lsp/QuarkusLanguageClient.java index 7219484b3..7bdb4f2c4 100644 --- a/src/main/java/com/redhat/devtools/intellij/quarkus/lsp/QuarkusLanguageClient.java +++ b/src/main/java/com/redhat/devtools/intellij/quarkus/lsp/QuarkusLanguageClient.java @@ -125,7 +125,9 @@ public CompletableFuture getJavaHover(MicroProfileJavaHoverParams javaPar @Override public CompletableFuture> getJavaDiagnostics(MicroProfileJavaDiagnosticsParams javaParams) { - return runAsBackground("Computing MicroProfile Java diagnostics", monitor -> PropertiesManagerForJava.getInstance().diagnostics(javaParams, PsiUtilsLSImpl.getInstance(getProject()))); + // When 'microprofile/java/diagnostics' is too many called + var coalesceBy = javaParams.getUris(); + return runAsBackground("Computing MicroProfile Java diagnostics", monitor -> PropertiesManagerForJava.getInstance().diagnostics(javaParams, PsiUtilsLSImpl.getInstance(getProject())), coalesceBy); } @Override diff --git a/src/main/java/com/redhat/devtools/intellij/qute/lsp/QuteLanguageClient.java b/src/main/java/com/redhat/devtools/intellij/qute/lsp/QuteLanguageClient.java index c77f7ca4e..ec44d4c7b 100644 --- a/src/main/java/com/redhat/devtools/intellij/qute/lsp/QuteLanguageClient.java +++ b/src/main/java/com/redhat/devtools/intellij/qute/lsp/QuteLanguageClient.java @@ -155,8 +155,9 @@ public CompletableFuture> getJavaCodelens(QuteJavaCodeL @Override public CompletableFuture> getJavaDiagnostics(QuteJavaDiagnosticsParams javaParams) { - return runAsBackground("getJavaDiagnostics", monitor -> QuteSupportForJava.getInstance().diagnostics(javaParams, PsiUtilsLSImpl.getInstance(getProject()), - monitor)); + // When 'microprofile/java/diagnostics' is too many called + var coalesceBy = javaParams.getUris(); + return runAsBackground("getJavaDiagnostics", monitor -> QuteSupportForJava.getInstance().diagnostics(javaParams, PsiUtilsLSImpl.getInstance(getProject()), monitor), coalesceBy); } @Override