From 993c1920d17f1072fe17654f9fe553983f58098d Mon Sep 17 00:00:00 2001
From: Vsevolod Tolstopyatov <qwwdfsad@gmail.com>
Date: Mon, 19 Oct 2020 03:01:50 -0700
Subject: [PATCH] =?UTF-8?q?Cleanup=20lazy=20coroutines=20that=20have=20bee?=
 =?UTF-8?q?n=20cancelled=20but=20not=20yet=20garbage=20=E2=80=A6=20(#2315)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Cleanup lazy coroutines that have been cancelled but not yet garbage collected

Fixes #2294
---
 .../jvm/src/debug/internal/DebugProbesImpl.kt | 23 ++++++++++++++++++-
 .../test/LazyCoroutineTest.kt                 | 23 +++++++++++++++++++
 2 files changed, 45 insertions(+), 1 deletion(-)
 create mode 100644 kotlinx-coroutines-debug/test/LazyCoroutineTest.kt

diff --git a/kotlinx-coroutines-core/jvm/src/debug/internal/DebugProbesImpl.kt b/kotlinx-coroutines-core/jvm/src/debug/internal/DebugProbesImpl.kt
index 9dd6c5a548..4b7c09b347 100644
--- a/kotlinx-coroutines-core/jvm/src/debug/internal/DebugProbesImpl.kt
+++ b/kotlinx-coroutines-core/jvm/src/debug/internal/DebugProbesImpl.kt
@@ -156,7 +156,11 @@ internal object DebugProbesImpl {
                 // Stable ordering of coroutines by their sequence number
                 .sortedBy { it.info.sequenceNumber }
                 // Leave in the dump only the coroutines that were not collected while we were dumping them
-                .mapNotNull { owner -> owner.info.context?.let { context -> create(owner, context) } }
+                .mapNotNull { owner ->
+                    // Fuse map and filter into one operation to save an inline
+                    if (owner.isFinished()) null
+                    else owner.info.context?.let { context -> create(owner, context) }
+                }
         }
 
     /*
@@ -183,10 +187,27 @@ internal object DebugProbesImpl {
         dumpCoroutinesSynchronized(out)
     }
 
+    /*
+     * Filters out coroutines that do not call probeCoroutineCompleted,
+     * are completed, but not yet garbage collected.
+     *
+     * Typically, we intercept completion of the coroutine so it invokes "probeCoroutineCompleted",
+     * but it's not the case for lazy coroutines that get cancelled before start.
+     */
+    private fun CoroutineOwner<*>.isFinished(): Boolean {
+        // Guarded by lock
+        val job = info.context?.get(Job) ?: return false
+        if (!job.isCompleted) return false
+        capturedCoroutinesMap.remove(this) // Clean it up by the way
+        return true
+    }
+
     private fun dumpCoroutinesSynchronized(out: PrintStream): Unit = coroutineStateLock.write {
         check(isInstalled) { "Debug probes are not installed" }
         out.print("Coroutines dump ${dateFormat.format(System.currentTimeMillis())}")
         capturedCoroutines
+            .asSequence()
+            .filter { !it.isFinished() }
             .sortedBy { it.info.sequenceNumber }
             .forEach { owner ->
                 val info = owner.info
diff --git a/kotlinx-coroutines-debug/test/LazyCoroutineTest.kt b/kotlinx-coroutines-debug/test/LazyCoroutineTest.kt
new file mode 100644
index 0000000000..c872b6a53d
--- /dev/null
+++ b/kotlinx-coroutines-debug/test/LazyCoroutineTest.kt
@@ -0,0 +1,23 @@
+/*
+ * Copyright 2016-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
+ */
+package kotlinx.coroutines.debug
+
+import kotlinx.coroutines.*
+import org.junit.Test
+import kotlin.test.*
+
+class LazyCoroutineTest : DebugTestBase() {
+
+    @Test
+    fun testLazyCompletedCoroutine() = runTest {
+        val job = launch(start = CoroutineStart.LAZY) {}
+        job.invokeOnCompletion { expect(2) }
+        expect(1)
+        job.cancelAndJoin()
+        expect(3)
+        assertEquals(1, DebugProbes.dumpCoroutinesInfo().size) // Outer runBlocking
+        verifyPartialDump(1, "BlockingCoroutine{Active}")
+        finish(4)
+    }
+}