Skip to content

Commit e463218

Browse files
committed
Fix a race in Job.join that sporadically results in normal completion
The race happens in the slow-path of 'join' implementation when parent invokes join on a child coroutines that crashes and cancels the parent. Fixes #1123
1 parent 6687406 commit e463218

File tree

2 files changed

+42
-0
lines changed

2 files changed

+42
-0
lines changed

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

+5
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,11 @@ internal open class CancellableContinuationImpl<in T>(
170170
// otherwise, onCompletionInternal was already invoked & invoked tryResume, and the result is in the state
171171
val state = this.state
172172
if (state is CompletedExceptionally) throw recoverStackTrace(state.cause, this)
173+
// if the parent job was already cancelled, then throw the corresponding cancellation exception
174+
// otherwise, there is a race is suspendCancellableCoroutine { cont -> ... } does cont.resume(...)
175+
// before the block returns. This getResult would return a result as opposed to cancellation
176+
// exception that should have happened if the continuation is dispatched for execution later.
177+
if (resumeMode == MODE_CANCELLABLE) context.checkCompletion()
173178
return getSuccessfulResult(state)
174179
}
175180

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/*
2+
* Copyright 2016-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
3+
*/
4+
5+
package kotlinx.coroutines
6+
7+
import org.junit.*
8+
9+
/**
10+
* Test a race between job failure and join.
11+
*
12+
* See [#1123](https://github.com/Kotlin/kotlinx.coroutines/issues/1123).
13+
*/
14+
class JobStructuredJoinStressTest : TestBase() {
15+
private val nRepeats = 10_000 * stressTestMultiplier
16+
17+
@Test
18+
fun testStress() {
19+
repeat(nRepeats) {
20+
assertFailsWith<TestException> {
21+
runBlocking {
22+
// launch in background
23+
val job = launch(Dispatchers.Default) {
24+
throw TestException("OK") // crash
25+
}
26+
try {
27+
job.join()
28+
} catch (e: CancellationException) {
29+
// expected -- proceed
30+
throw e
31+
}
32+
error("job.join returned normally, but it should not have")
33+
}
34+
}
35+
}
36+
}
37+
}

0 commit comments

Comments
 (0)