From b6c1f2ed4b379ff5e26a12cce005f341a16a2864 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Tue, 6 Sep 2022 13:12:37 +0200 Subject: [PATCH 1/3] Fix cancelled delayed jobs not being disposed of in TestDispatcher Fixes #3398 --- .../common/src/TestDispatcher.kt | 9 +++++- .../jvm/test/MemoryLeakTest.kt | 28 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 kotlinx-coroutines-test/jvm/test/MemoryLeakTest.kt diff --git a/kotlinx-coroutines-test/common/src/TestDispatcher.kt b/kotlinx-coroutines-test/common/src/TestDispatcher.kt index 348cc2f185..39e517d4ed 100644 --- a/kotlinx-coroutines-test/common/src/TestDispatcher.kt +++ b/kotlinx-coroutines-test/common/src/TestDispatcher.kt @@ -31,7 +31,14 @@ public abstract class TestDispatcher internal constructor() : CoroutineDispatche /** @suppress */ override fun scheduleResumeAfterDelay(timeMillis: Long, continuation: CancellableContinuation) { val timedRunnable = CancellableContinuationRunnable(continuation, this) - scheduler.registerEvent(this, timeMillis, timedRunnable, continuation.context, ::cancellableRunnableIsCancelled) + val handle = scheduler.registerEvent( + this, + timeMillis, + timedRunnable, + continuation.context, + ::cancellableRunnableIsCancelled + ) + continuation.invokeOnCancellation { handle.dispose() } } /** @suppress */ diff --git a/kotlinx-coroutines-test/jvm/test/MemoryLeakTest.kt b/kotlinx-coroutines-test/jvm/test/MemoryLeakTest.kt new file mode 100644 index 0000000000..54ec929adf --- /dev/null +++ b/kotlinx-coroutines-test/jvm/test/MemoryLeakTest.kt @@ -0,0 +1,28 @@ +/* + * Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ +import kotlinx.coroutines.* +import kotlinx.coroutines.test.* +import java.lang.ref.* +import kotlin.test.* + +class MemoryLeakTest { + + @Test + fun testCancellationLeakInTestCoroutineScheduler() = runTest { + lateinit var weakRef: WeakReference<*> + val job = launch(start = CoroutineStart.UNDISPATCHED) { + val leakingObject = Any() + weakRef = WeakReference(leakingObject) + delay(3) + // This code is needed to hold a reference to `leakingObject` until the job itself is weakly reachable. + leakingObject.hashCode() + } + job.cancel() + System.gc() + assertNotNull(weakRef.get()) // the cancellation handler has not run yet + runCurrent() + System.gc() + assertNull(weakRef.get()) // the cancellation handler has run, disposing of the job + } +} From e707bb97e012292fb2bc365229fe2230096a21b8 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Wed, 7 Sep 2022 11:18:25 +0200 Subject: [PATCH 2/3] Use FieldWalker instead of System.gc for the test --- kotlinx-coroutines-test/jvm/test/MemoryLeakTest.kt | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/kotlinx-coroutines-test/jvm/test/MemoryLeakTest.kt b/kotlinx-coroutines-test/jvm/test/MemoryLeakTest.kt index 54ec929adf..705c97eae4 100644 --- a/kotlinx-coroutines-test/jvm/test/MemoryLeakTest.kt +++ b/kotlinx-coroutines-test/jvm/test/MemoryLeakTest.kt @@ -3,26 +3,21 @@ */ import kotlinx.coroutines.* import kotlinx.coroutines.test.* -import java.lang.ref.* import kotlin.test.* class MemoryLeakTest { @Test fun testCancellationLeakInTestCoroutineScheduler() = runTest { - lateinit var weakRef: WeakReference<*> + val leakingObject = Any() val job = launch(start = CoroutineStart.UNDISPATCHED) { - val leakingObject = Any() - weakRef = WeakReference(leakingObject) - delay(3) + delay(1) // This code is needed to hold a reference to `leakingObject` until the job itself is weakly reachable. leakingObject.hashCode() } job.cancel() - System.gc() - assertNotNull(weakRef.get()) // the cancellation handler has not run yet + FieldWalker.assertReachableCount(1, testScheduler) { it === leakingObject } runCurrent() - System.gc() - assertNull(weakRef.get()) // the cancellation handler has run, disposing of the job + FieldWalker.assertReachableCount(0, testScheduler) { it === leakingObject } } } From fcf8c5f3ebaeb5ba44dfab77841ce0ff12fdaa32 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Wed, 7 Sep 2022 11:52:36 +0200 Subject: [PATCH 3/3] ~ --- kotlinx-coroutines-test/common/src/TestDispatcher.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kotlinx-coroutines-test/common/src/TestDispatcher.kt b/kotlinx-coroutines-test/common/src/TestDispatcher.kt index 39e517d4ed..9616bb0b23 100644 --- a/kotlinx-coroutines-test/common/src/TestDispatcher.kt +++ b/kotlinx-coroutines-test/common/src/TestDispatcher.kt @@ -38,7 +38,7 @@ public abstract class TestDispatcher internal constructor() : CoroutineDispatche continuation.context, ::cancellableRunnableIsCancelled ) - continuation.invokeOnCancellation { handle.dispose() } + continuation.disposeOnCancellation(handle) } /** @suppress */