diff --git a/kotlinx-coroutines-core/common/src/Builders.common.kt b/kotlinx-coroutines-core/common/src/Builders.common.kt index d7a66fe3de..26fd8e7050 100644 --- a/kotlinx-coroutines-core/common/src/Builders.common.kt +++ b/kotlinx-coroutines-core/common/src/Builders.common.kt @@ -156,7 +156,7 @@ public suspend fun withContext( } } // SLOW PATH -- use new dispatcher - val coroutine = DispatchedCoroutine(newContext, uCont) // MODE_ATOMIC_DEFAULT + val coroutine = DispatchedCoroutine(newContext, uCont) // MODE_CANCELLABLE coroutine.initParentJob() block.startCoroutineCancellable(coroutine, coroutine) coroutine.getResult() @@ -215,7 +215,7 @@ private class DispatchedCoroutine( context: CoroutineContext, uCont: Continuation ) : ScopeCoroutine(context, uCont) { - override val defaultResumeMode: Int get() = MODE_ATOMIC_DEFAULT + override val defaultResumeMode: Int get() = MODE_CANCELLABLE // this is copy-and-paste of a decision state machine inside AbstractionContinuation // todo: we may some-how abstract it via inline class diff --git a/kotlinx-coroutines-core/common/src/Dispatched.kt b/kotlinx-coroutines-core/common/src/Dispatched.kt index b767cc135d..450163c668 100644 --- a/kotlinx-coroutines-core/common/src/Dispatched.kt +++ b/kotlinx-coroutines-core/common/src/Dispatched.kt @@ -218,32 +218,35 @@ internal abstract class DispatchedTask( public final override fun run() { val taskContext = this.taskContext - var exception: Throwable? = null + var fatalException: Throwable? = null try { val delegate = delegate as DispatchedContinuation val continuation = delegate.continuation val context = continuation.context - val job = if (resumeMode.isCancellableMode) context[Job] else null val state = takeState() // NOTE: Must take state in any case, even if cancelled withCoroutineContext(context, delegate.countOrElement) { - if (job != null && !job.isActive) { + val exception = getExceptionalResult(state) + val job = if (resumeMode.isCancellableMode) context[Job] else null + /* + * Check whether continuation was originally resumed with an exception. + * If so, it dominates cancellation, otherwise the original exception + * will be silently lost. + */ + if (exception == null && job != null && !job.isActive) { val cause = job.getCancellationException() cancelResult(state, cause) continuation.resumeWithStackTrace(cause) } else { - val exception = getExceptionalResult(state) - if (exception != null) - continuation.resumeWithStackTrace(exception) - else - continuation.resume(getSuccessfulResult(state)) + if (exception != null) continuation.resumeWithStackTrace(exception) + else continuation.resume(getSuccessfulResult(state)) } } } catch (e: Throwable) { // This instead of runCatching to have nicer stacktrace and debug experience - exception = e + fatalException = e } finally { val result = runCatching { taskContext.afterTask() } - handleFatalException(exception, result.exceptionOrNull()) + handleFatalException(fatalException, result.exceptionOrNull()) } } diff --git a/kotlinx-coroutines-core/common/test/TestBase.common.kt b/kotlinx-coroutines-core/common/test/TestBase.common.kt index 449d958073..cef74c1704 100644 --- a/kotlinx-coroutines-core/common/test/TestBase.common.kt +++ b/kotlinx-coroutines-core/common/test/TestBase.common.kt @@ -74,3 +74,5 @@ public fun wrapperDispatcher(context: CoroutineContext): CoroutineContext { } } +public suspend fun wrapperDispatcher(): CoroutineContext = wrapperDispatcher(coroutineContext) + diff --git a/kotlinx-coroutines-core/common/test/WithContextTest.kt b/kotlinx-coroutines-core/common/test/WithContextTest.kt index be6bde4a09..55127e5c0b 100644 --- a/kotlinx-coroutines-core/common/test/WithContextTest.kt +++ b/kotlinx-coroutines-core/common/test/WithContextTest.kt @@ -15,7 +15,7 @@ class WithContextTest : TestBase() { fun testThrowException() = runTest { expect(1) try { - withContext(coroutineContext) { + withContext(coroutineContext) { expect(2) throw AssertionError() } @@ -31,7 +31,7 @@ class WithContextTest : TestBase() { fun testThrowExceptionFromWrappedContext() = runTest { expect(1) try { - withContext(wrapperDispatcher(coroutineContext)) { + withContext(wrapperDispatcher(coroutineContext)) { expect(2) throw AssertionError() } @@ -151,7 +151,7 @@ class WithContextTest : TestBase() { expect(2) try { // Same dispatcher, different context - withContext(CoroutineName("testRunCancellationUndispatchedVsException")) { + withContext(CoroutineName("testRunCancellationUndispatchedVsException")) { expect(3) yield() // must suspend expect(5) @@ -176,7 +176,7 @@ class WithContextTest : TestBase() { expect(2) try { // "Different" dispatcher (schedules execution back and forth) - withContext(wrapperDispatcher(coroutineContext)) { + withContext(wrapperDispatcher(coroutineContext)) { expect(4) yield() // must suspend expect(6) @@ -204,7 +204,7 @@ class WithContextTest : TestBase() { job = launch(Job()) { try { expect(3) - withContext(wrapperDispatcher(coroutineContext)) { + withContext(wrapperDispatcher(coroutineContext)) { require(isActive) expect(5) job!!.cancel() @@ -349,6 +349,26 @@ class WithContextTest : TestBase() { expectUnreached() } + @Test + fun testSequentialCancellation() = runTest { + val job = launch { + expect(1) + withContext(wrapperDispatcher()) { + expect(2) + } + expectUnreached() + } + + yield() + val job2 = launch { + expect(3) + job.cancel() + } + + joinAll(job, job2) + finish(4) + } + private class Wrapper(val value: String) : Incomplete { override val isActive: Boolean get() = error("")