do not run checkCanceled hooks in too low-level place to fix IDEA-268788 Indexing...
authorAlexey Kudravtsev <cdr@intellij.com>
Fri, 7 May 2021 13:08:47 +0000 (15:08 +0200)
committerintellij-monorepo-bot <intellij-monorepo-bot-no-reply@jetbrains.com>
Fri, 7 May 2021 13:47:29 +0000 (13:47 +0000)
GitOrigin-RevId: a0d25fb5806daa4f627d0dec9ac09507500f558b

platform/platform-impl/src/com/intellij/openapi/application/impl/ReadMostlyRWLock.java
platform/platform-tests/testSrc/com/intellij/openapi/progress/impl/ProgressIndicatorTest.java

index 2221919d5381d14fdc5298675f42ec1c5bbb8a55..571b27d337028afc742fb82349f459b3982456d5 100644 (file)
@@ -3,6 +3,8 @@ package com.intellij.openapi.application.impl;
 
 import com.intellij.openapi.application.AccessToken;
 import com.intellij.openapi.application.ex.ApplicationUtil;
+import com.intellij.openapi.progress.ProcessCanceledException;
+import com.intellij.openapi.progress.ProgressIndicator;
 import com.intellij.openapi.progress.ProgressManager;
 import com.intellij.openapi.progress.impl.CoreProgressManager;
 import com.intellij.util.containers.ConcurrentList;
@@ -103,12 +105,15 @@ final class ReadMostlyRWLock {
     if (tryReadLock(status)) {
       return;
     }
+    ProgressIndicator progress = ProgressManager.getGlobalProgressIndicator();
     for (int iter = 0; ; iter++) {
       if (tryReadLock(status)) {
         break;
       }
-
-      ProgressManager.checkCanceled();
+      // do not run any checkCanceled hooks to avoid deadlock
+      if (progress != null && progress.isCanceled()) {
+        throw new ProcessCanceledException();
+      }
       waitABit(status, iter);
     }
   }
index 67afa7cf2dd4c04e14e6dbe4aa49669cde3a3ba4..49686e009d4de12bf5e00d023a0c173bba19440b 100644 (file)
@@ -14,6 +14,7 @@ import com.intellij.openapi.application.impl.LaterInvocator;
 import com.intellij.openapi.diagnostic.DefaultLogger;
 import com.intellij.openapi.progress.*;
 import com.intellij.openapi.progress.util.*;
+import com.intellij.openapi.util.Disposer;
 import com.intellij.openapi.util.EmptyRunnable;
 import com.intellij.openapi.wm.ex.ProgressIndicatorEx;
 import com.intellij.testFramework.BombedProgressIndicator;
@@ -1067,4 +1068,52 @@ public class ProgressIndicatorTest extends LightPlatformTestCase {
 
     assertThrows(Exception.class, () -> new ProgressIndicatorBase().stop());
   }
+
+  public void testComplexCheckCanceledHookDoesntInterfereWithReadLockAcquire() throws Exception {
+    AtomicBoolean futureEntered = new AtomicBoolean();
+    AtomicBoolean futureExited = new AtomicBoolean();
+    AtomicBoolean readActionCompleted = new AtomicBoolean();
+    CoreProgressManager.CheckCanceledHook hook = __ -> {
+      doReadAction(); // in case this hook gets called during read action, it will eventually SOE
+      return false;
+    };
+    ((ProgressManagerImpl)ProgressManager.getInstance()).addCheckCanceledHook(hook);
+    Disposer.register(getTestRootDisposable(), ()->((ProgressManagerImpl)ProgressManager.getInstance()).removeCheckCanceledHook(hook));
+
+    DefaultLogger.disableStderrDumping(getTestRootDisposable());
+    ProgressIndicator indicator = new ProgressIndicatorBase(false);
+    indicator.start();
+
+    ApplicationManager.getApplication().assertIsDispatchThread();
+
+    AtomicReference<Future<?>> future = new AtomicReference<>();
+    doReadAction();
+
+    WriteAction.run(() -> {
+      future.set(ApplicationManager.getApplication().executeOnPooledThread(() -> {
+         ProgressManager.getInstance().runProcess(() -> {
+           futureEntered.set(true);
+           doReadAction();
+           readActionCompleted.set(true);
+           futureExited.set(true);
+         }, indicator);
+      }));
+      while (!futureEntered.get()) {
+        // wait until hook is called and finish write action
+      }
+      TimeoutUtil.sleep(10_000); // ensure to be inside read action by now
+    });
+    while (!futureExited.get()) {
+
+    }
+    future.get().get();
+    assertTrue(readActionCompleted.get());
+  }
+
+  // extracted to separate method to avoid re-compilation on hot path taking unpredictable time, blowing timeouts
+  private static void doReadAction() {
+    ReadAction.run(() -> {
+      //`
+    });
+  }
 }