fix: Too many non-blocking read actions submitted at once in

LSPDiagnosticHandler Fixes redhat-developer#1089 Signed-off-by: azerr <[email protected]>
angelozerr · Aug 10, 2023 · 43f45f2 · 43f45f2
1 parent bd549b4
commit 43f45f2
Show file tree

Hide file tree

Showing 7 changed files with 60 additions and 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
@@ -39,7 +39,19 @@ public IndexAwareLanguageClient(Project project) {
      * @return the output of the function code
      */
     protected <R> CompletableFuture<R> runAsBackground(String progressTitle, Function<ProgressIndicator, R> 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 <R>   the return type
+     * @return the output of the function code
+     */
+    protected <R> CompletableFuture<R> runAsBackground(String progressTitle, Function<ProgressIndicator, R> 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
@@ -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<ResultOrError<R>> nonBlockingReadActionPromise;
 
-    public LSPCompletableFuture(Function<ProgressIndicator, R> code, String progressTitle, IndexAwareLanguageClient languageClient) {
+    public LSPCompletableFuture(Function<ProgressIndicator, R> 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<ResultOrError<R>> nonBlockingReadActionPromise(boolea
         if (executeInSmartMode) {
             action = action.inSmartMode(project);
         }
+        if (coalesceBy != null) {
+            action = action.coalesceBy(coalesceBy);
+        }
         return action
                 .submit(AppExecutorUtil.getAppExecutorService());
     }

diff --git a/...java/com/redhat/devtools/intellij/lsp4ij/operations/diagnostics/LSPDiagnosticHandler.java b/...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/...va/com/redhat/devtools/intellij/lsp4mp4ij/classpath/ClasspathResourceChangedNotifier.java b/...va/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<Pair<VirtualFile, Module>> sourceFiles;
     private boolean librariesChanged;
-private final List<RunnableProgress> processBeforeLibrariesChanged;
+    private final List<RunnableProgress> processBeforeLibrariesChanged;
+    private boolean disposed;
 
     public ClasspathResourceChangedNotifier(Project project, List<RunnableProgress> preprocessors) {
         this.project = project;
@@ -74,6 +75,9 @@ public synchronized void addSourceFile(Pair<VirtualFile, Module> 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/.../redhat/devtools/intellij/lsp4mp4ij/psi/internal/jaxrs/java/DefaultJaxRsInfoProvider.java b/.../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<JaxRsMethodInfo> getJaxRsMethodInfo(PsiFile typeRoot, JaxRsContext j
 		List<JaxRsMethodInfo> 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
@@ -125,7 +125,9 @@ public CompletableFuture<Hover> getJavaHover(MicroProfileJavaHoverParams javaPar
 
     @Override
     public CompletableFuture<List<PublishDiagnosticsParams>> 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
@@ -155,8 +155,9 @@ public CompletableFuture<List<? extends CodeLens>> getJavaCodelens(QuteJavaCodeL
 
   @Override
   public CompletableFuture<List<PublishDiagnosticsParams>> 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