Skip to content

Commit fbd41e3

Browse files
committed
Check cancellation on the undispatched resume
* It allows to apply the same reasoning about cancellation as in dispatched coroutines * Makes cancellation modes consistent Related to #1177
1 parent c81dc91 commit fbd41e3

File tree

4 files changed

+72
-46
lines changed

4 files changed

+72
-46
lines changed

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

+18-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,24 @@ internal fun <T> Continuation<T>.resumeUninterceptedMode(value: T, mode: Int) {
4343
MODE_ATOMIC_DEFAULT -> intercepted().resume(value)
4444
MODE_CANCELLABLE -> intercepted().resumeCancellable(value)
4545
MODE_DIRECT -> resume(value)
46-
MODE_UNDISPATCHED -> withCoroutineContext(context, null) { resume(value) }
46+
MODE_UNDISPATCHED -> withCoroutineContext(context, null) {
47+
/*
48+
* This check is necessary to make a cancellable resume when coroutine's context was changed,
49+
* but it dispatcher wasn't.
50+
* For example, in the situation like this:
51+
* ```
52+
* withContext(Job()) {
53+
* // Here we know that outer job is cancelled
54+
* } // <- then here CE should occur
55+
* ```
56+
*/
57+
val job = context[Job]
58+
if (job != null && !job.isActive) {
59+
resumeWithException(job.getCancellationException())
60+
} else {
61+
resume(value)
62+
}
63+
}
4764
MODE_IGNORE -> {}
4865
else -> error("Invalid mode $mode")
4966
}

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -299,15 +299,15 @@ class CoroutinesTest : TestBase() {
299299
} finally {
300300
expect(5)
301301
withContext(NonCancellable) { yield() } // to test
302-
expect(7)
302+
expectUnreached()
303303
}
304304
expectUnreached() // will get cancelled, because parent crashes
305305
}
306306
expect(3)
307307
yield() // to parent
308308
expect(6)
309309
parent.join() // make sure crashed parent still waits for its child
310-
finish(8)
310+
finish(7)
311311
// make sure is cancelled
312312
assertTrue(child.isCancelled)
313313
}

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

+2-43
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,6 @@ class NonCancellableTest : TestBase() {
1616
yield()
1717
expect(4)
1818
}
19-
20-
expect(5)
21-
yield()
2219
expectUnreached()
2320
}
2421

@@ -36,7 +33,7 @@ class NonCancellableTest : TestBase() {
3633
} else {
3734
assertNull(e.cause)
3835
}
39-
finish(6)
36+
finish(5)
4037
}
4138
}
4239

@@ -49,9 +46,6 @@ class NonCancellableTest : TestBase() {
4946
yield()
5047
expect(4)
5148
}
52-
53-
expect(5)
54-
yield()
5549
expectUnreached()
5650
}
5751

@@ -64,7 +58,7 @@ class NonCancellableTest : TestBase() {
6458
expectUnreached()
6559
} catch (e: TestCancellationException) {
6660
assertEquals("TEST", e.message)
67-
finish(6)
61+
finish(5)
6862
}
6963
}
7064

@@ -99,39 +93,4 @@ class NonCancellableTest : TestBase() {
9993
finish(6)
10094
}
10195
}
102-
103-
@Test
104-
fun testNonCancellableTwice() = runTest {
105-
expect(1)
106-
val job = async {
107-
withContext(NonCancellable) {
108-
expect(2)
109-
yield()
110-
expect(4)
111-
}
112-
113-
withContext(NonCancellable) {
114-
expect(5)
115-
yield()
116-
expect(6)
117-
}
118-
}
119-
120-
yield()
121-
job.cancel()
122-
expect(3)
123-
assertTrue(job.isCancelled)
124-
try {
125-
job.await()
126-
expectUnreached()
127-
} catch (e: JobCancellationException) {
128-
if (RECOVER_STACK_TRACES) {
129-
val cause = e.cause as JobCancellationException // shall be recovered JCE
130-
assertNull(cause.cause)
131-
} else {
132-
assertNull(e.cause)
133-
}
134-
finish(7)
135-
}
136-
}
13796
}

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

+50
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
package kotlinx.coroutines
99

10+
import kotlinx.coroutines.channels.*
1011
import kotlin.test.*
1112

1213
class WithContextTest : TestBase() {
@@ -369,6 +370,55 @@ class WithContextTest : TestBase() {
369370
finish(4)
370371
}
371372

373+
@Test
374+
fun testSequentialCancellationUndispatched() = runTest {
375+
val latch = Channel<Unit>(1)
376+
val job = launch {
377+
expect(2)
378+
withContext(NonCancellable) {
379+
expect(3)
380+
latch.receive()
381+
expect(5)
382+
}
383+
expectUnreached()
384+
}
385+
386+
expect(1)
387+
yield()
388+
expect(4)
389+
job.cancel()
390+
latch.send(Unit)
391+
yield()
392+
finish(6)
393+
}
394+
395+
@Test
396+
fun testExceptionWithSequentialCancellationUndispatched() = runTest {
397+
val latch = Channel<Unit>(1)
398+
val job = launch {
399+
expect(2)
400+
try {
401+
withContext<Int>(NonCancellable) {
402+
expect(3)
403+
latch.receive()
404+
expect(5)
405+
throw TestException()
406+
}
407+
expectUnreached()
408+
} catch (e: TestException) {
409+
expect(6)
410+
}
411+
}
412+
413+
expect(1)
414+
yield()
415+
expect(4)
416+
job.cancel()
417+
latch.send(Unit)
418+
yield()
419+
finish(7)
420+
}
421+
372422
private class Wrapper(val value: String) : Incomplete {
373423
override val isActive: Boolean
374424
get() = error("")

0 commit comments

Comments
 (0)