Skip to content

Commit 510ccfd

Browse files
committed
Fix possible leaking of CompletedExceptionally on racy cancellation
1 parent 78832e3 commit 510ccfd

File tree

4 files changed

+37
-13
lines changed

4 files changed

+37
-13
lines changed

common/kotlinx-coroutines-core-common/src/Timeout.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ private fun <U, T: U> setupTimeout(
7474
coroutine.disposeOnCompletion(context.delay.invokeOnTimeout(coroutine.time, coroutine))
7575
// restart block using new coroutine with new job,
7676
// however start it as undispatched coroutine, because we are already in the proper context
77-
return coroutine.startUndispatchedOrReturnIgnoreTimeout(coroutine, block, coroutine)
77+
return coroutine.startUndispatchedOrReturnIgnoreTimeout(coroutine, block)
7878
}
7979

8080
private open class TimeoutCoroutine<U, in T: U>(

common/kotlinx-coroutines-core-common/src/intrinsics/Undispatched.kt

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -80,19 +80,20 @@ private inline fun <T> startDirect(completion: Continuation<T>, block: () -> Any
8080
*/
8181
internal fun <T, R> AbstractCoroutine<T>.startUndispatchedOrReturn(receiver: R, block: suspend R.() -> T): Any? {
8282
initParentJob()
83-
return undispatchedResult({ true }) { block.startCoroutineUninterceptedOrReturn(receiver, this) }
83+
return undispatchedResult({ true }) {
84+
block.startCoroutineUninterceptedOrReturn(receiver, this)
85+
}
8486
}
8587

8688
/**
8789
* Same as [startUndispatchedOrReturn], but ignores [TimeoutCancellationException] on fast-path.
8890
*/
8991
internal fun <T, R> AbstractCoroutine<T>.startUndispatchedOrReturnIgnoreTimeout(
90-
receiver: R, block: suspend R.() -> T,
91-
timeoutCoroutine: Job
92-
): Any? {
92+
receiver: R, block: suspend R.() -> T): Any? {
9393
initParentJob()
94-
return undispatchedResult({ e -> !(e is TimeoutCancellationException && e.coroutine === timeoutCoroutine) })
95-
{ block.startCoroutineUninterceptedOrReturn(receiver, this) }
94+
return undispatchedResult({ e -> !(e is TimeoutCancellationException && e.coroutine === this) }) {
95+
block.startCoroutineUninterceptedOrReturn(receiver, this)
96+
}
9697
}
9798

9899
private inline fun <T> AbstractCoroutine<T>.undispatchedResult(
@@ -124,10 +125,10 @@ private inline fun <T> AbstractCoroutine<T>.undispatchedResult(
124125
makeCompletingOnce(result, MODE_IGNORE) -> {
125126
val state = state
126127
if (state is CompletedExceptionally) {
127-
if (shouldThrow(state.cause)) {
128-
throw state.cause
129-
} else {
130-
result
128+
when {
129+
shouldThrow(state.cause) -> throw state.cause
130+
result is CompletedExceptionally -> throw result.cause
131+
else -> result
131132
}
132133
} else {
133134
state

common/kotlinx-coroutines-core-common/test/WithTimeoutOrNullTest.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,4 +238,4 @@ class WithTimeoutOrNullTest : TestBase() {
238238
finish(4)
239239
}
240240
}
241-
}
241+
}

core/kotlinx-coroutines-core/test/WithTimeoutOrNullJvmTest.kt

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ class WithTimeoutOrNullJvmTest : TestBase() {
2424

2525
@Test
2626
fun testIgnoredTimeout() = runTest {
27-
val value =withTimeout(1) {
27+
val value = withTimeout(1) {
2828
Thread.sleep(10)
2929
42
3030
}
@@ -41,4 +41,27 @@ class WithTimeoutOrNullJvmTest : TestBase() {
4141

4242
assertEquals(42, value)
4343
}
44+
45+
@Test
46+
fun testIgnoredTimeoutOnNullThrowsCancellation() = runTest {
47+
try {
48+
withTimeoutOrNull(1) {
49+
expect(1)
50+
Thread.sleep(10)
51+
throw CancellationException()
52+
}
53+
expectUnreached()
54+
} catch (e: CancellationException) {
55+
finish(2)
56+
}
57+
}
58+
59+
@Test
60+
fun testIgnoredTimeoutOnNullThrowsOnYield() = runTest {
61+
val value = withTimeoutOrNull(1) {
62+
Thread.sleep(10)
63+
yield()
64+
}
65+
assertNull(value)
66+
}
4467
}

0 commit comments

Comments
 (0)