Skip to content

Commit 0f94304

Browse files
elizarovqwwdfsad
authored andcommitted
Fix lost exception from dispatched withContext on cancelled job
Fixes #675
1 parent 0f26f66 commit 0f94304

File tree

4 files changed

+104
-2
lines changed

4 files changed

+104
-2
lines changed

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ public suspend fun <T> withContext(
214214
}
215215
}
216216
// SLOW PATH -- use new dispatcher
217-
val coroutine = DispatchedCoroutine(newContext, uCont) // MODE_DISPATCHED
217+
val coroutine = DispatchedCoroutine(newContext, uCont) // MODE_ATOMIC_DEFAULT
218218
coroutine.initParentJob()
219219
block.startCoroutineCancellable(coroutine, coroutine)
220220
coroutine.getResult()
@@ -292,7 +292,7 @@ private class DispatchedCoroutine<in T>(
292292
context: CoroutineContext,
293293
uCont: Continuation<T>
294294
) : ScopeCoroutine<T>(context, uCont) {
295-
override val defaultResumeMode: Int get() = MODE_CANCELLABLE
295+
override val defaultResumeMode: Int get() = MODE_ATOMIC_DEFAULT
296296

297297
// this is copy-and-paste of a decision state machine inside AbstractionContinuation
298298
// todo: we may some-how abstract it via inline class

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

+24
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,30 @@ class CoroutineScopeTest : TestBase() {
219219
}
220220
}
221221

222+
@Test
223+
fun testCoroutineScopeCancellationVsException() = runTest {
224+
expect(1)
225+
var job: Job? = null
226+
job = launch(start = CoroutineStart.UNDISPATCHED) {
227+
expect(2)
228+
try {
229+
coroutineScope {
230+
expect(3)
231+
yield() // must suspend
232+
expect(5)
233+
job!!.cancel() // cancel this job _before_ it throws
234+
throw TestException1()
235+
}
236+
} catch (e: TestException1) {
237+
// must have caught TextException
238+
expect(6)
239+
}
240+
}
241+
expect(4)
242+
yield() // to coroutineScope
243+
finish(7)
244+
}
245+
222246
@Test
223247
fun testScopePlusContext() {
224248
assertSame(EmptyCoroutineContext, scopePlusContext(EmptyCoroutineContext, EmptyCoroutineContext))

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

+24
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,30 @@ class SupervisorTest : TestBase() {
195195
assertTrue(parent.isCancelled)
196196
}
197197

198+
@Test
199+
fun testSupervisorScopeCancellationVsException() = runTest {
200+
expect(1)
201+
var job: Job? = null
202+
job = launch(start = CoroutineStart.UNDISPATCHED) {
203+
expect(2)
204+
try {
205+
supervisorScope {
206+
expect(3)
207+
yield() // must suspend
208+
expect(5)
209+
job!!.cancel() // cancel this job _before_ it throws
210+
throw TestException1()
211+
}
212+
} catch (e: TestException1) {
213+
// must have caught TextException
214+
expect(6)
215+
}
216+
}
217+
expect(4)
218+
yield() // to coroutineScope
219+
finish(7)
220+
}
221+
198222
private class TestException1 : Exception()
199223
private class TestException2 : Exception()
200224
}

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

+54
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,60 @@ class WithContextTest : TestBase() {
139139
}
140140
}
141141

142+
@Test
143+
fun testRunCancellationUndispatchedVsException() = runTest {
144+
expect(1)
145+
var job: Job? = null
146+
job = launch(start = CoroutineStart.UNDISPATCHED) {
147+
expect(2)
148+
try {
149+
// Same dispatcher, different context
150+
withContext(CoroutineName("testRunCancellationUndispatchedVsException")) {
151+
expect(3)
152+
yield() // must suspend
153+
expect(5)
154+
job!!.cancel() // cancel this job _before_ it throws
155+
throw TestException()
156+
}
157+
} catch (e: TestException) {
158+
// must have caught TextException
159+
expect(6)
160+
}
161+
}
162+
expect(4)
163+
yield() // to coroutineScope
164+
finish(7)
165+
}
166+
167+
@Test
168+
fun testRunCancellationDispatchedVsException() = runTest {
169+
expect(1)
170+
var job: Job? = null
171+
job = launch(start = CoroutineStart.UNDISPATCHED) {
172+
expect(2)
173+
try {
174+
// "Different" dispatcher (schedules execution back and forth)
175+
withContext(wrapperDispatcher(coroutineContext)) {
176+
expect(4)
177+
yield() // must suspend
178+
expect(6)
179+
job!!.cancel() // cancel this job _before_ it throws
180+
throw TestException()
181+
}
182+
} catch (e: TestException) {
183+
// must have caught TextException
184+
expect(8)
185+
}
186+
}
187+
expect(3)
188+
yield() // withContext is next
189+
expect(5)
190+
yield() // withContext again
191+
expect(7)
192+
yield() // to catch block
193+
finish(9)
194+
}
195+
142196
@Test
143197
fun testRunSelfCancellationWithException() = runTest(unhandled = listOf({e -> e is AssertionError})) {
144198
expect(1)

0 commit comments

Comments
 (0)