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

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

Hide file tree

Showing 8 changed files with 241 additions and 135 deletions.
diff --git a/src/main/java/com/redhat/devtools/intellij/lsp4ij/client/CoalesceByKey.java b/src/main/java/com/redhat/devtools/intellij/lsp4ij/client/CoalesceByKey.java
@@ -0,0 +1,41 @@
+/*******************************************************************************
+ * Copyright (c) 2023 Red Hat, Inc.
+ * Distributed under license by Red Hat, Inc. All rights reserved.
+ * This program is made available under the terms of the
+ * Eclipse Public License v2.0 which accompanies this distribution,
+ * and is available at https://www.eclipse.org/legal/epl-v20.html
+ *
+ * Contributors:
+ * Red Hat, Inc. - initial API and implementation
+ ******************************************************************************/
+package com.redhat.devtools.intellij.lsp4ij.client;
+
+import java.util.Objects;
+
+/**
+ * A CoalesceBy key which defines a type (like LSP request) and a value which should be identified the request.
+ */
+public class CoalesceByKey {
+
+    private final String type;
+
+    private final Object value;
+
+    public CoalesceByKey(String type, Object value) {
+        this.type = type;
+        this.value = value;
+    }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (o == null || getClass() != o.getClass()) return false;
+        CoalesceByKey that = (CoalesceByKey) o;
+        return Objects.equals(type, that.type) && Objects.equals(value, that.value);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(type, value);
+    }
+}
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;
@@ -25,6 +26,7 @@
 import com.redhat.devtools.intellij.lsp4ij.LSPIJUtils;
 import com.redhat.devtools.intellij.lsp4ij.LSPVirtualFileWrapper;
 import com.redhat.devtools.intellij.lsp4ij.LanguageServerWrapper;
+import com.redhat.devtools.intellij.lsp4ij.client.CoalesceByKey;
 import org.eclipse.lsp4j.PublishDiagnosticsParams;
 
 import java.util.function.Consumer;
@@ -45,12 +47,23 @@ 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());
-
+            // Cancel if needed the previous "textDocument/publishDiagnostics" for a given uri.
+            var coalesceBy = new CoalesceByKey("textDocument/publishDiagnostics", params.getUri());
+            var executeInSmartMode = DumbService.getInstance(languageServerWrapper.getProject()).isDumb();
+            var action = ReadAction.nonBlocking(() -> updateDiagnostics(params))
+                    .expireWith(languageServerWrapper)
+                    .coalesceBy(coalesceBy);
+            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,13 +140,23 @@ public void run(@NotNull ProgressIndicator progressIndicator) {
         }
     }
 
+    public boolean isDisposed() {
+        return disposed;
+    }
+
     @Override
     public void dispose() {
+        if (isDisposed()) {
+            return;
+        }
+        this.disposed = true;
         if (debounceTask != null) {
             debounceTask.cancel();
+            debounceTask =null;
         }
-        if(debounceTimer != null) {
+        if (debounceTimer != null) {
             debounceTimer.cancel();
+            debounceTimer = null;
         }
     }
 }
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
@@ -15,6 +15,7 @@
 import com.intellij.openapi.util.Pair;
 import com.intellij.openapi.vfs.VirtualFile;
 import com.intellij.util.messages.MessageBusConnection;
+import com.redhat.devtools.intellij.lsp4ij.client.CoalesceByKey;
 import com.redhat.devtools.intellij.lsp4mp4ij.psi.core.ProjectLabelManager;
 import com.redhat.devtools.intellij.lsp4mp4ij.psi.core.PropertiesManager;
 import com.redhat.devtools.intellij.lsp4mp4ij.psi.core.PropertiesManagerForJava;
@@ -125,7 +126,13 @@ 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 project is indexing and user types a lot of characters in the Java editor, the MicroProfile language server
+        // validates the Java document and consumes a 'microprofile/java/diagnostics' for each typed character.
+        // The response of 'microprofile/java/diagnostics' are blocked (or takes some times) and we have
+        // "Too many non-blocking read actions submitted at once in". To avoid having this error, we create a coalesceBy key
+        // managed by IJ ReadAction.nonBlocking() to cancel the previous request.
+        var coalesceBy = new CoalesceByKey("microprofile/java/diagnostics", javaParams.getUris());
+        return runAsBackground("Computing MicroProfile Java diagnostics", monitor -> PropertiesManagerForJava.getInstance().diagnostics(javaParams, PsiUtilsLSImpl.getInstance(getProject())), coalesceBy);
     }
 
     @Override