From 81d6e89f6cf9f297cf25f58e32cd23c5b64d23c4 Mon Sep 17 00:00:00 2001
From: peter <peter@jetbrains.com>
Date: Tue, 29 Oct 2019 19:03:57 +0100
Subject: [PATCH] fix leaks via NonBlockingReadAction#coalesceBy: don't
 remember tasks if they're already canceled and won't be removed

GitOrigin-RevId: 1242ef740e36ff8364160264fcce567c70b4cd81
---
 .../impl/NonBlockingReadActionImpl.java       |  8 +++++-
 .../impl/NonBlockingReadActionTest.java       | 26 +++++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/platform/platform-impl/src/com/intellij/openapi/application/impl/NonBlockingReadActionImpl.java b/platform/platform-impl/src/com/intellij/openapi/application/impl/NonBlockingReadActionImpl.java
index 5d31398d99f8..1a16d421eb26 100644
--- a/platform/platform-impl/src/com/intellij/openapi/application/impl/NonBlockingReadActionImpl.java
+++ b/platform/platform-impl/src/com/intellij/openapi/application/impl/NonBlockingReadActionImpl.java
@@ -229,7 +229,7 @@ public class NonBlockingReadActionImpl<T>
     }
 
     private void scheduleReplacementIfAny() {
-      if (myReplacement == null) {
+      if (myReplacement == null || myReplacement.promise.isDone()) {
         ourTasksByEquality.remove(myCoalesceEquality, this);
       } else {
         ourTasksByEquality.put(myCoalesceEquality, myReplacement);
@@ -239,6 +239,8 @@ public class NonBlockingReadActionImpl<T>
 
     void submitOrScheduleCoalesced(@NotNull List<Object> coalesceEquality) {
       synchronized (ourTasksByEquality) {
+        if (promise.isDone()) return;
+
         NonBlockingReadActionImpl<?>.Submission current = ourTasksByEquality.get(coalesceEquality);
         if (current == null) {
           ourTasksByEquality.put(coalesceEquality, this);
@@ -441,4 +443,8 @@ public class NonBlockingReadActionImpl<T>
     }
   }
 
+  @TestOnly
+  static Map<List<Object>, NonBlockingReadActionImpl<?>.Submission> getTasksByEquality() {
+    return ourTasksByEquality;
+  }
 }
diff --git a/platform/platform-tests/testSrc/com/intellij/openapi/application/impl/NonBlockingReadActionTest.java b/platform/platform-tests/testSrc/com/intellij/openapi/application/impl/NonBlockingReadActionTest.java
index 42ff08787308..292df9fe5dc0 100644
--- a/platform/platform-tests/testSrc/com/intellij/openapi/application/impl/NonBlockingReadActionTest.java
+++ b/platform/platform-tests/testSrc/com/intellij/openapi/application/impl/NonBlockingReadActionTest.java
@@ -1,6 +1,7 @@
 // Copyright 2000-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
 package com.intellij.openapi.application.impl;
 
+import com.intellij.openapi.Disposable;
 import com.intellij.openapi.application.ModalityState;
 import com.intellij.openapi.application.NonBlockingReadAction;
 import com.intellij.openapi.application.ReadAction;
@@ -10,8 +11,10 @@ import com.intellij.openapi.editor.impl.DocumentImpl;
 import com.intellij.openapi.progress.EmptyProgressIndicator;
 import com.intellij.openapi.progress.ProgressIndicator;
 import com.intellij.openapi.progress.ProgressManager;
+import com.intellij.openapi.util.Disposer;
 import com.intellij.openapi.util.Pair;
 import com.intellij.psi.util.PsiUtilCore;
+import com.intellij.testFramework.LeakHunter;
 import com.intellij.testFramework.LightPlatformTestCase;
 import com.intellij.util.concurrency.AppExecutorUtil;
 import com.intellij.util.concurrency.Semaphore;
@@ -176,4 +179,27 @@ public class NonBlockingReadActionTest extends LightPlatformTestCase {
     }
     waitForPromise(promise);
   }
+
+  public void testDoNotLeakFirstCancelledCoalescedAction() {
+    Object leak = new Object(){};
+    Disposable disposable = Disposer.newDisposable();
+    Disposer.dispose(disposable);
+    CancellablePromise<String> p = ReadAction
+      .nonBlocking(() -> "a")
+      .expireWith(disposable)
+      .coalesceBy(leak)
+      .submit(AppExecutorUtil.getAppExecutorService());
+    assertTrue(p.isCancelled());
+    LeakHunter.checkLeak(NonBlockingReadActionImpl.getTasksByEquality(), leak.getClass());
+  }
+
+  public void testDoNotLeakSecondCancelledCoalescedAction() {
+    Object leak = new Object(){};
+    CancellablePromise<String> p = ReadAction.nonBlocking(() -> "a").coalesceBy(leak).submit(AppExecutorUtil.getAppExecutorService());
+    WriteAction.run(() -> {
+      ReadAction.nonBlocking(() -> "b").coalesceBy(leak).submit(AppExecutorUtil.getAppExecutorService()).cancel();
+    });
+    assertTrue(p.isDone());
+    LeakHunter.checkLeak(NonBlockingReadActionImpl.getTasksByEquality(), leak.getClass());
+  }
 }