Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup lazy coroutines that have been cancelled but not yet garbage … #2315

Merged
merged 2 commits into from Oct 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -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) }
}
}

/*
Expand All @@ -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
Expand Down
23 changes: 23 additions & 0 deletions 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)
}
}