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 · 882e475 · 882e475
1 parent bd549b4
commit 882e475
Show file tree

Hide file tree

Showing 8 changed files with 276 additions and 146 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,44 @@
+/*******************************************************************************
+ * 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.Arrays;
+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) && Arrays.equals(value, that.value);
+    }
+
+    @Override
+    public int hashCode() {
+        int result = Objects.hash(type);
+        result = 31 * result + Arrays.hashCode(value);
+        return result;
+    }
+}
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;
@@ -112,65 +113,81 @@ private boolean isConfigSource(VirtualFile file) {
 
     @Override
     public CompletableFuture<MicroProfileProjectInfo> getProjectInfo(MicroProfileProjectInfoParams params) {
+        var coalesceBy = new CoalesceByKey("microprofile/projectInfo", params.getUri(), params.getScopes());
         String filePath = getFilePath(params.getUri());
         return runAsBackground("Computing MicroProfile properties for '" + filePath + "'.", monitor ->
                 PropertiesManager.getInstance().getMicroProfileProjectInfo(params, PsiUtilsLSImpl.getInstance(getProject()), monitor)
-        );
+        , coalesceBy);
     }
 
     @Override
     public CompletableFuture<Hover> getJavaHover(MicroProfileJavaHoverParams javaParams) {
-        return runAsBackground("Computing MicroProfile Java hover", monitor -> PropertiesManagerForJava.getInstance().hover(javaParams, PsiUtilsLSImpl.getInstance(getProject())));
+        var coalesceBy = new CoalesceByKey("microprofile/java/hover", javaParams.getUri(), javaParams.getPosition());
+        return runAsBackground("Computing MicroProfile Java hover", monitor -> PropertiesManagerForJava.getInstance().hover(javaParams, PsiUtilsLSImpl.getInstance(getProject())), coalesceBy);
     }
 
     @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
     public CompletableFuture<Location> getPropertyDefinition(MicroProfilePropertyDefinitionParams params) {
-        return runAsBackground("Computing property definition", monitor -> PropertiesManager.getInstance().findPropertyLocation(params, PsiUtilsLSImpl.getInstance(getProject())));
+        var coalesceBy = new CoalesceByKey("microprofile/propertyDefinition", params.getUri(), params.getSourceType(), params.getSourceField(), params.getSourceMethod());
+        return runAsBackground("Computing property definition", monitor -> PropertiesManager.getInstance().findPropertyLocation(params, PsiUtilsLSImpl.getInstance(getProject())), coalesceBy);
     }
 
     @Override
     public CompletableFuture<ProjectLabelInfoEntry> getJavaProjectLabels(MicroProfileJavaProjectLabelsParams javaParams) {
-        return runAsBackground("Computing Java projects labels", monitor -> ProjectLabelManager.getInstance().getProjectLabelInfo(javaParams, PsiUtilsLSImpl.getInstance(getProject())));
+        var coalesceBy = new CoalesceByKey("microprofile/java/projectLabels", javaParams.getUri(), javaParams.getTypes());
+        return runAsBackground("Computing Java projects labels", monitor -> ProjectLabelManager.getInstance().getProjectLabelInfo(javaParams, PsiUtilsLSImpl.getInstance(getProject())), coalesceBy);
     }
 
     @Override
     public CompletableFuture<List<ProjectLabelInfoEntry>> getAllJavaProjectLabels() {
-        return runAsBackground("Computing All Java projects labels", monitor -> ProjectLabelManager.getInstance().getProjectLabelInfo(PsiUtilsLSImpl.getInstance(getProject())));
+        var coalesceBy = new CoalesceByKey("microprofile/java/workspaceLabels");
+        return runAsBackground("Computing All Java projects labels", monitor -> ProjectLabelManager.getInstance().getProjectLabelInfo(PsiUtilsLSImpl.getInstance(getProject())),coalesceBy);
     }
 
     @Override
     public CompletableFuture<JavaFileInfo> getJavaFileInfo(MicroProfileJavaFileInfoParams javaParams) {
-        return runAsBackground("Computing Java file info", monitor -> PropertiesManagerForJava.getInstance().fileInfo(javaParams, PsiUtilsLSImpl.getInstance(getProject())));
+        var coalesceBy = new CoalesceByKey("microprofile/java/fileInfo", javaParams.getUri());
+        return runAsBackground("Computing Java file info", monitor -> PropertiesManagerForJava.getInstance().fileInfo(javaParams, PsiUtilsLSImpl.getInstance(getProject())), coalesceBy);
     }
 
     @Override
     public CompletableFuture<List<MicroProfileDefinition>> getJavaDefinition(MicroProfileJavaDefinitionParams javaParams) {
-        return runAsBackground("Computing Java definitions", monitor -> PropertiesManagerForJava.getInstance().definition(javaParams, PsiUtilsLSImpl.getInstance(getProject())));
+        var coalesceBy = new CoalesceByKey("microprofile/java/definition", javaParams.getUri(),javaParams.getPosition());
+        return runAsBackground("Computing Java definitions", monitor -> PropertiesManagerForJava.getInstance().definition(javaParams, PsiUtilsLSImpl.getInstance(getProject())), coalesceBy);
     }
 
     @Override
     public CompletableFuture<MicroProfileJavaCompletionResult> getJavaCompletion(MicroProfileJavaCompletionParams javaParams) {
+        var coalesceBy = new CoalesceByKey("microprofile/java/completion", javaParams.getUri(),javaParams.getPosition());
         return runAsBackground("Computing Java completion", monitor -> {
             IPsiUtils utils = PsiUtilsLSImpl.getInstance(getProject());
             CompletionList completionList = PropertiesManagerForJava.getInstance().completion(javaParams, utils);
             JavaCursorContextResult cursorContext = PropertiesManagerForJava.getInstance().javaCursorContext(javaParams, utils);
             return new MicroProfileJavaCompletionResult(completionList, cursorContext);
-        });
+        }, coalesceBy);
     }
 
     @Override
     public CompletableFuture<List<? extends CodeLens>> getJavaCodelens(MicroProfileJavaCodeLensParams javaParams) {
-        return runAsBackground("Computing Java codelens", monitor -> PropertiesManagerForJava.getInstance().codeLens(javaParams, PsiUtilsLSImpl.getInstance(getProject()), monitor));
+        var coalesceBy = new CoalesceByKey("microprofile/java/codeLens", javaParams.getUri());
+        return runAsBackground("Computing Java codelens", monitor -> PropertiesManagerForJava.getInstance().codeLens(javaParams, PsiUtilsLSImpl.getInstance(getProject()), monitor), coalesceBy);
     }
 
     @Override
     public CompletableFuture<List<CodeAction>> getJavaCodeAction(MicroProfileJavaCodeActionParams javaParams) {
-        return runAsBackground("Computing Java code actions", monitor -> (List<CodeAction>) PropertiesManagerForJava.getInstance().codeAction(javaParams, PsiUtilsLSImpl.getInstance(getProject())));
+        var coalesceBy = new CoalesceByKey("microprofile/java/codeAction", javaParams.getUri());
+        return runAsBackground("Computing Java code actions", monitor -> (List<CodeAction>) PropertiesManagerForJava.getInstance().codeAction(javaParams, PsiUtilsLSImpl.getInstance(getProject())),coalesceBy);
     }
 
     @Override
@@ -184,7 +201,8 @@ public CompletableFuture<CodeAction> resolveCodeAction(CodeAction unresolved) {
 
     @Override
     public CompletableFuture<JavaCursorContextResult> getJavaCursorContext(MicroProfileJavaCompletionParams params) {
-        return runAsBackground("Computing Java Cursor context", monitor -> PropertiesManagerForJava.getInstance().javaCursorContext(params, PsiUtilsLSImpl.getInstance(getProject())));
+        var coalesceBy = new CoalesceByKey("microprofile/java/javaCursorContext", params.getUri(), params.getPosition());
+        return runAsBackground("Computing Java Cursor context", monitor -> PropertiesManagerForJava.getInstance().javaCursorContext(params, PsiUtilsLSImpl.getInstance(getProject())), coalesceBy);
     }
 
     @Override