From 5a296fd665e65122d5bb3c7f151daa79f26c01c0 Mon Sep 17 00:00:00 2001 From: Maksim Zuev Date: Wed, 17 Jul 2024 17:19:01 +0200 Subject: [PATCH 01/11] Do not report exceptions raised in CoroutineDispatcher.dispatch as internal errors --- .../src/internal/DispatchedContinuation.kt | 12 ++- .../common/src/internal/DispatchedTask.kt | 11 +++ .../jvm/test/ExecutorsTest.kt | 75 +++++++++++++++++++ 3 files changed, 96 insertions(+), 2 deletions(-) diff --git a/kotlinx-coroutines-core/common/src/internal/DispatchedContinuation.kt b/kotlinx-coroutines-core/common/src/internal/DispatchedContinuation.kt index 26e7c5abd4..996866881d 100644 --- a/kotlinx-coroutines-core/common/src/internal/DispatchedContinuation.kt +++ b/kotlinx-coroutines-core/common/src/internal/DispatchedContinuation.kt @@ -190,7 +190,7 @@ internal class DispatchedContinuation( if (dispatcher.isDispatchNeeded(context)) { _state = state resumeMode = MODE_ATOMIC - dispatcher.dispatch(context, this) + dispatchWithExceptionHandling(context) } else { executeUnconfined(state, MODE_ATOMIC) { withCoroutineContext(context, countOrElement) { @@ -208,7 +208,7 @@ internal class DispatchedContinuation( if (dispatcher.isDispatchNeeded(context)) { _state = state resumeMode = MODE_CANCELLABLE - dispatcher.dispatch(context, this) + dispatchWithExceptionHandling(context) } else { executeUnconfined(state, MODE_CANCELLABLE) { if (!resumeCancelled(state)) { @@ -247,6 +247,14 @@ internal class DispatchedContinuation( override fun toString(): String = "DispatchedContinuation[$dispatcher, ${continuation.toDebugString()}]" + + private fun dispatchWithExceptionHandling(context: CoroutineContext) { + try { + dispatcher.dispatch(context, this) + } catch (e: Throwable) { + throw DispatchException(e) + } + } } /** diff --git a/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt b/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt index 309685bb7c..b909bd8b79 100644 --- a/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt +++ b/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt @@ -77,6 +77,7 @@ internal abstract class DispatchedTask internal constructor( final override fun run() { assert { resumeMode != MODE_UNINITIALIZED } // should have been set before dispatching var fatalException: Throwable? = null + var dispatchException: DispatchException? = null try { val delegate = delegate as DispatchedContinuation val continuation = delegate.continuation @@ -102,11 +103,14 @@ internal abstract class DispatchedTask internal constructor( } } } + } catch (e: DispatchException) { + dispatchException = e } catch (e: Throwable) { // This instead of runCatching to have nicer stacktrace and debug experience fatalException = e } finally { fatalException?.let { handleFatalException(it) } + dispatchException?.let { handleCoroutineException(delegate.context, it.cause!!) } } } @@ -205,3 +209,10 @@ internal inline fun DispatchedTask<*>.runUnconfinedEventLoop( internal inline fun Continuation<*>.resumeWithStackTrace(exception: Throwable) { resumeWith(Result.failure(recoverStackTrace(exception, this))) } + +/** + * This exception holds an exception raised in [CoroutineDispatcher.dispatch] method + * + * @see DispatchedContinuation.dispatchWithExceptionHandling + */ +internal class DispatchException(cause: Throwable) : Exception(cause) diff --git a/kotlinx-coroutines-core/jvm/test/ExecutorsTest.kt b/kotlinx-coroutines-core/jvm/test/ExecutorsTest.kt index 1ad2f8a2a4..8639f021bc 100644 --- a/kotlinx-coroutines-core/jvm/test/ExecutorsTest.kt +++ b/kotlinx-coroutines-core/jvm/test/ExecutorsTest.kt @@ -119,4 +119,79 @@ class ExecutorsTest : TestBase() { dispatcher.close() check(executorService.isShutdown) } + + @Test + fun testEarlyExecutorShutdown() { + runTestExceptionInDispatch(6, { it is RejectedExecutionException }) { + expect(1) + val dispatcher = newSingleThreadContext("Ctx") + launch(dispatcher) { + withContext(Dispatchers.Default) { + expect(2) + delay(100) + expect(4) + } + } + + delay(50) + expect(3) + + dispatcher.close() + } + } + + @Test + fun testExceptionInDispatch() { + runTestExceptionInDispatch(5, { it is TestException }) { + val dispatcher = object : CoroutineDispatcher() { + private var closed = false + override fun dispatch(context: CoroutineContext, block: Runnable) { + if (closed) throw TestException() + Dispatchers.Default.dispatch(context, block) + } + + fun close() { + closed = true + } + } + launch(dispatcher) { + withContext(Dispatchers.Default) { + expect(1) + delay(100) + expect(3) + } + } + + delay(50) + expect(2) + dispatcher.close() + } + } + + private fun runTestExceptionInDispatch( + totalSteps: Int, + isExpectedException: (Throwable) -> Boolean, + block: suspend CoroutineScope.() -> Unit, + ) { + var mainThread: Thread? = null + val exceptionHandler = CoroutineExceptionHandler { _, e -> + if (isExpectedException(e)) { + expect(totalSteps - 1) + mainThread!!.run { + interrupt() + unpark(this) + } + } else { + expectUnreached() + } + } + try { + runBlocking(exceptionHandler) { + block() + mainThread = Thread.currentThread() + } + } catch (_: InterruptedException) { + finish(totalSteps) + } + } } From 5dc0f5aec2e71d7ff3126b4a941c7109506f458a Mon Sep 17 00:00:00 2001 From: Maksim Zuev Date: Wed, 17 Jul 2024 18:26:47 +0200 Subject: [PATCH 02/11] fixup! Do not report exceptions raised in CoroutineDispatcher.dispatch as internal errors Prevent reporting DispatchException itself --- .../common/src/CoroutineExceptionHandler.kt | 7 ++++--- .../common/src/intrinsics/Cancellable.kt | 5 +++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/kotlinx-coroutines-core/common/src/CoroutineExceptionHandler.kt b/kotlinx-coroutines-core/common/src/CoroutineExceptionHandler.kt index e6f1d9e63c..7f6dbdfd12 100644 --- a/kotlinx-coroutines-core/common/src/CoroutineExceptionHandler.kt +++ b/kotlinx-coroutines-core/common/src/CoroutineExceptionHandler.kt @@ -16,18 +16,19 @@ import kotlin.coroutines.* */ @InternalCoroutinesApi public fun handleCoroutineException(context: CoroutineContext, exception: Throwable) { + val reportException = if (exception is DispatchException) exception.cause!! else exception // Invoke an exception handler from the context if present try { context[CoroutineExceptionHandler]?.let { - it.handleException(context, exception) + it.handleException(context, reportException) return } } catch (t: Throwable) { - handleUncaughtCoroutineException(context, handlerException(exception, t)) + handleUncaughtCoroutineException(context, handlerException(reportException, t)) return } // If a handler is not present in the context or an exception was thrown, fallback to the global handler - handleUncaughtCoroutineException(context, exception) + handleUncaughtCoroutineException(context, reportException) } internal fun handlerException(originalException: Throwable, thrownException: Throwable): Throwable { diff --git a/kotlinx-coroutines-core/common/src/intrinsics/Cancellable.kt b/kotlinx-coroutines-core/common/src/intrinsics/Cancellable.kt index 2f9a434a1f..83d185c50c 100644 --- a/kotlinx-coroutines-core/common/src/intrinsics/Cancellable.kt +++ b/kotlinx-coroutines-core/common/src/intrinsics/Cancellable.kt @@ -58,6 +58,7 @@ private fun dispatcherFailure(completion: Continuation<*>, e: Throwable) { * 2) Rethrow the exception immediately, so it will crash the caller (e.g. when the coroutine had * no parent or it was async/produce over MainScope). */ - completion.resumeWith(Result.failure(e)) - throw e + val reportException = if (e is DispatchException) e.cause!! else e + completion.resumeWith(Result.failure(reportException)) + throw reportException } From 53513dea7129ad79fdd8bb576e140ce8cdab6cb5 Mon Sep 17 00:00:00 2001 From: Maksim Zuev Date: Thu, 18 Jul 2024 10:01:06 +0200 Subject: [PATCH 03/11] fixup! Do not report exceptions raised in CoroutineDispatcher.dispatch as internal errors Fix test: add new exception to the list of known exceptions --- .../kotlin/ListAllCoroutineThrowableSubclassesTest.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/integration-testing/src/jvmCoreTest/kotlin/ListAllCoroutineThrowableSubclassesTest.kt b/integration-testing/src/jvmCoreTest/kotlin/ListAllCoroutineThrowableSubclassesTest.kt index 5c564c8a8e..0cf592c636 100644 --- a/integration-testing/src/jvmCoreTest/kotlin/ListAllCoroutineThrowableSubclassesTest.kt +++ b/integration-testing/src/jvmCoreTest/kotlin/ListAllCoroutineThrowableSubclassesTest.kt @@ -26,6 +26,7 @@ class ListAllCoroutineThrowableSubclassesTest { "kotlinx.coroutines.internal.DiagnosticCoroutineContextException", "kotlinx.coroutines.internal.ExceptionSuccessfullyProcessed", "kotlinx.coroutines.CoroutinesInternalError", + "kotlinx.coroutines.DispatchException", "kotlinx.coroutines.channels.ClosedSendChannelException", "kotlinx.coroutines.channels.ClosedReceiveChannelException", "kotlinx.coroutines.flow.internal.ChildCancelledException", From cb207e568439997ace02f817fd92c559a8fd6073 Mon Sep 17 00:00:00 2001 From: Maksim Zuev Date: Mon, 2 Sep 2024 10:40:27 +0200 Subject: [PATCH 04/11] Review: save val cause --- kotlinx-coroutines-core/common/src/CoroutineExceptionHandler.kt | 2 +- kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt | 2 +- kotlinx-coroutines-core/common/src/intrinsics/Cancellable.kt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/kotlinx-coroutines-core/common/src/CoroutineExceptionHandler.kt b/kotlinx-coroutines-core/common/src/CoroutineExceptionHandler.kt index 7f6dbdfd12..0899eb6fb6 100644 --- a/kotlinx-coroutines-core/common/src/CoroutineExceptionHandler.kt +++ b/kotlinx-coroutines-core/common/src/CoroutineExceptionHandler.kt @@ -16,7 +16,7 @@ import kotlin.coroutines.* */ @InternalCoroutinesApi public fun handleCoroutineException(context: CoroutineContext, exception: Throwable) { - val reportException = if (exception is DispatchException) exception.cause!! else exception + val reportException = if (exception is DispatchException) exception.cause else exception // Invoke an exception handler from the context if present try { context[CoroutineExceptionHandler]?.let { diff --git a/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt b/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt index b909bd8b79..0e22023097 100644 --- a/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt +++ b/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt @@ -110,7 +110,7 @@ internal abstract class DispatchedTask internal constructor( fatalException = e } finally { fatalException?.let { handleFatalException(it) } - dispatchException?.let { handleCoroutineException(delegate.context, it.cause!!) } + dispatchException?.let { handleCoroutineException(delegate.context, it.cause) } } } diff --git a/kotlinx-coroutines-core/common/src/intrinsics/Cancellable.kt b/kotlinx-coroutines-core/common/src/intrinsics/Cancellable.kt index 83d185c50c..1e87d767af 100644 --- a/kotlinx-coroutines-core/common/src/intrinsics/Cancellable.kt +++ b/kotlinx-coroutines-core/common/src/intrinsics/Cancellable.kt @@ -58,7 +58,7 @@ private fun dispatcherFailure(completion: Continuation<*>, e: Throwable) { * 2) Rethrow the exception immediately, so it will crash the caller (e.g. when the coroutine had * no parent or it was async/produce over MainScope). */ - val reportException = if (e is DispatchException) e.cause!! else e + val reportException = if (e is DispatchException) e.cause else e completion.resumeWith(Result.failure(reportException)) throw reportException } From 7bd5fbef884b9b0596bcf240d11ba53a65f86ff9 Mon Sep 17 00:00:00 2001 From: Maksim Zuev Date: Mon, 2 Sep 2024 10:40:58 +0200 Subject: [PATCH 05/11] Report dispatcher and context --- .../common/src/internal/DispatchedContinuation.kt | 2 +- .../common/src/internal/DispatchedTask.kt | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/kotlinx-coroutines-core/common/src/internal/DispatchedContinuation.kt b/kotlinx-coroutines-core/common/src/internal/DispatchedContinuation.kt index 996866881d..8b5ee12479 100644 --- a/kotlinx-coroutines-core/common/src/internal/DispatchedContinuation.kt +++ b/kotlinx-coroutines-core/common/src/internal/DispatchedContinuation.kt @@ -252,7 +252,7 @@ internal class DispatchedContinuation( try { dispatcher.dispatch(context, this) } catch (e: Throwable) { - throw DispatchException(e) + throw DispatchException(e, dispatcher, context) } } } diff --git a/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt b/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt index 0e22023097..24d6c67a4e 100644 --- a/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt +++ b/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt @@ -215,4 +215,8 @@ internal inline fun Continuation<*>.resumeWithStackTrace(exception: Throwable) { * * @see DispatchedContinuation.dispatchWithExceptionHandling */ -internal class DispatchException(cause: Throwable) : Exception(cause) +internal class DispatchException( + override val cause: Throwable, + dispatcher: CoroutineDispatcher, + context: CoroutineContext, +) : Exception("Coroutine dispatcher $dispatcher threw an exception, context = $context", cause) From 5befa1d7c51365d92b804952937ab4e39f32b5c7 Mon Sep 17 00:00:00 2001 From: Maksim Zuev Date: Mon, 2 Sep 2024 11:17:43 +0200 Subject: [PATCH 06/11] Protect isDispatchNeeded calls --- .../common/src/CoroutineDispatcher.kt | 2 +- kotlinx-coroutines-core/common/src/Yield.kt | 2 +- .../src/internal/DispatchedContinuation.kt | 28 ++++++++++++------- .../common/src/internal/DispatchedTask.kt | 6 ++-- .../common/src/internal/LimitedDispatcher.kt | 6 ++-- .../common/src/internal/NamedDispatcher.kt | 4 +-- kotlinx-coroutines-core/jvm/src/Executors.kt | 4 +-- 7 files changed, 30 insertions(+), 22 deletions(-) diff --git a/kotlinx-coroutines-core/common/src/CoroutineDispatcher.kt b/kotlinx-coroutines-core/common/src/CoroutineDispatcher.kt index 37b68760a0..b208c84f83 100644 --- a/kotlinx-coroutines-core/common/src/CoroutineDispatcher.kt +++ b/kotlinx-coroutines-core/common/src/CoroutineDispatcher.kt @@ -225,7 +225,7 @@ public abstract class CoroutineDispatcher : * @suppress **This an internal API and should not be used from general code.** */ @InternalCoroutinesApi - public open fun dispatchYield(context: CoroutineContext, block: Runnable): Unit = dispatch(context, block) + public open fun dispatchYield(context: CoroutineContext, block: Runnable): Unit = safeDispatch(context, block) /** * Returns a continuation that wraps the provided [continuation], thus intercepting all resumptions. diff --git a/kotlinx-coroutines-core/common/src/Yield.kt b/kotlinx-coroutines-core/common/src/Yield.kt index 0598228640..afe79494b7 100644 --- a/kotlinx-coroutines-core/common/src/Yield.kt +++ b/kotlinx-coroutines-core/common/src/Yield.kt @@ -26,7 +26,7 @@ public suspend fun yield(): Unit = suspendCoroutineUninterceptedOrReturn sc@ { u val context = uCont.context context.ensureActive() val cont = uCont.intercepted() as? DispatchedContinuation ?: return@sc Unit - if (cont.dispatcher.isDispatchNeeded(context)) { + if (cont.dispatcher.safeIsDispatchNeeded(context)) { // this is a regular dispatcher -- do simple dispatchYield cont.dispatchYield(context, Unit) } else { diff --git a/kotlinx-coroutines-core/common/src/internal/DispatchedContinuation.kt b/kotlinx-coroutines-core/common/src/internal/DispatchedContinuation.kt index 8b5ee12479..4c8f54e877 100644 --- a/kotlinx-coroutines-core/common/src/internal/DispatchedContinuation.kt +++ b/kotlinx-coroutines-core/common/src/internal/DispatchedContinuation.kt @@ -187,10 +187,10 @@ internal class DispatchedContinuation( override fun resumeWith(result: Result) { val state = result.toState() - if (dispatcher.isDispatchNeeded(context)) { + if (dispatcher.safeIsDispatchNeeded(context)) { _state = state resumeMode = MODE_ATOMIC - dispatchWithExceptionHandling(context) + dispatcher.safeDispatch(context, this) } else { executeUnconfined(state, MODE_ATOMIC) { withCoroutineContext(context, countOrElement) { @@ -205,10 +205,10 @@ internal class DispatchedContinuation( @Suppress("NOTHING_TO_INLINE") internal inline fun resumeCancellableWith(result: Result) { val state = result.toState() - if (dispatcher.isDispatchNeeded(context)) { + if (dispatcher.safeIsDispatchNeeded(context)) { _state = state resumeMode = MODE_CANCELLABLE - dispatchWithExceptionHandling(context) + dispatcher.safeDispatch(context, this) } else { executeUnconfined(state, MODE_CANCELLABLE) { if (!resumeCancelled(state)) { @@ -247,13 +247,21 @@ internal class DispatchedContinuation( override fun toString(): String = "DispatchedContinuation[$dispatcher, ${continuation.toDebugString()}]" +} - private fun dispatchWithExceptionHandling(context: CoroutineContext) { - try { - dispatcher.dispatch(context, this) - } catch (e: Throwable) { - throw DispatchException(e, dispatcher, context) - } +internal fun CoroutineDispatcher.safeDispatch(context: CoroutineContext, runnable: Runnable) { + try { + dispatch(context, runnable) + } catch (e: Throwable) { + throw DispatchException(e, this, context) + } +} + +internal fun CoroutineDispatcher.safeIsDispatchNeeded(context: CoroutineContext): Boolean { + try { + return isDispatchNeeded(context) + } catch (e: Throwable) { + throw DispatchException(e, this, context) } } diff --git a/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt b/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt index 24d6c67a4e..61e5902bab 100644 --- a/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt +++ b/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt @@ -147,8 +147,8 @@ internal fun DispatchedTask.dispatch(mode: Int) { // dispatch directly using this instance's Runnable implementation val dispatcher = delegate.dispatcher val context = delegate.context - if (dispatcher.isDispatchNeeded(context)) { - dispatcher.dispatch(context, this) + if (dispatcher.safeIsDispatchNeeded(context)) { + dispatcher.safeDispatch(context, this) } else { resumeUnconfined() } @@ -213,7 +213,7 @@ internal inline fun Continuation<*>.resumeWithStackTrace(exception: Throwable) { /** * This exception holds an exception raised in [CoroutineDispatcher.dispatch] method * - * @see DispatchedContinuation.dispatchWithExceptionHandling + * @see safeDispatch */ internal class DispatchException( override val cause: Throwable, diff --git a/kotlinx-coroutines-core/common/src/internal/LimitedDispatcher.kt b/kotlinx-coroutines-core/common/src/internal/LimitedDispatcher.kt index eb5196144f..488331fc37 100644 --- a/kotlinx-coroutines-core/common/src/internal/LimitedDispatcher.kt +++ b/kotlinx-coroutines-core/common/src/internal/LimitedDispatcher.kt @@ -42,7 +42,7 @@ internal class LimitedDispatcher( override fun dispatch(context: CoroutineContext, block: Runnable) { dispatchInternal(block) { worker -> - dispatcher.dispatch(this, worker) + dispatcher.safeDispatch(this, worker) } } @@ -116,10 +116,10 @@ internal class LimitedDispatcher( } currentTask = obtainTaskOrDeallocateWorker() ?: return // 16 is our out-of-thin-air constant to emulate fairness. Used in JS dispatchers as well - if (++fairnessCounter >= 16 && dispatcher.isDispatchNeeded(this@LimitedDispatcher)) { + if (++fairnessCounter >= 16 && dispatcher.safeIsDispatchNeeded(this@LimitedDispatcher)) { // Do "yield" to let other views execute their runnable as well // Note that we do not decrement 'runningWorkers' as we are still committed to our part of work - dispatcher.dispatch(this@LimitedDispatcher, this) + dispatcher.safeDispatch(this@LimitedDispatcher, this) return } } diff --git a/kotlinx-coroutines-core/common/src/internal/NamedDispatcher.kt b/kotlinx-coroutines-core/common/src/internal/NamedDispatcher.kt index 72dbd65380..deac600ef7 100644 --- a/kotlinx-coroutines-core/common/src/internal/NamedDispatcher.kt +++ b/kotlinx-coroutines-core/common/src/internal/NamedDispatcher.kt @@ -12,9 +12,9 @@ internal class NamedDispatcher( private val name: String ) : CoroutineDispatcher(), Delay by (dispatcher as? Delay ?: DefaultDelay) { - override fun isDispatchNeeded(context: CoroutineContext): Boolean = dispatcher.isDispatchNeeded(context) + override fun isDispatchNeeded(context: CoroutineContext): Boolean = dispatcher.safeIsDispatchNeeded(context) - override fun dispatch(context: CoroutineContext, block: Runnable) = dispatcher.dispatch(context, block) + override fun dispatch(context: CoroutineContext, block: Runnable) = dispatcher.safeDispatch(context, block) @InternalCoroutinesApi override fun dispatchYield(context: CoroutineContext, block: Runnable) = dispatcher.dispatchYield(context, block) diff --git a/kotlinx-coroutines-core/jvm/src/Executors.kt b/kotlinx-coroutines-core/jvm/src/Executors.kt index 8ba3f18a24..bdfbe6dbbc 100644 --- a/kotlinx-coroutines-core/jvm/src/Executors.kt +++ b/kotlinx-coroutines-core/jvm/src/Executors.kt @@ -105,8 +105,8 @@ public fun CoroutineDispatcher.asExecutor(): Executor = private class DispatcherExecutor(@JvmField val dispatcher: CoroutineDispatcher) : Executor { override fun execute(block: Runnable) { - if (dispatcher.isDispatchNeeded(EmptyCoroutineContext)) { - dispatcher.dispatch(EmptyCoroutineContext, block) + if (dispatcher.safeIsDispatchNeeded(EmptyCoroutineContext)) { + dispatcher.safeDispatch(EmptyCoroutineContext, block) } else { block.run() } From 5cff3ccfeac628134349e9f02c185b6fc45b3daa Mon Sep 17 00:00:00 2001 From: Maksim Zuev Date: Mon, 2 Sep 2024 11:32:08 +0200 Subject: [PATCH 07/11] Revert unnecessary wraps --- .../common/src/internal/NamedDispatcher.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kotlinx-coroutines-core/common/src/internal/NamedDispatcher.kt b/kotlinx-coroutines-core/common/src/internal/NamedDispatcher.kt index deac600ef7..72dbd65380 100644 --- a/kotlinx-coroutines-core/common/src/internal/NamedDispatcher.kt +++ b/kotlinx-coroutines-core/common/src/internal/NamedDispatcher.kt @@ -12,9 +12,9 @@ internal class NamedDispatcher( private val name: String ) : CoroutineDispatcher(), Delay by (dispatcher as? Delay ?: DefaultDelay) { - override fun isDispatchNeeded(context: CoroutineContext): Boolean = dispatcher.safeIsDispatchNeeded(context) + override fun isDispatchNeeded(context: CoroutineContext): Boolean = dispatcher.isDispatchNeeded(context) - override fun dispatch(context: CoroutineContext, block: Runnable) = dispatcher.safeDispatch(context, block) + override fun dispatch(context: CoroutineContext, block: Runnable) = dispatcher.dispatch(context, block) @InternalCoroutinesApi override fun dispatchYield(context: CoroutineContext, block: Runnable) = dispatcher.dispatchYield(context, block) From 1fbe6eb78d781c6a868fd8a9a7f7cd330f45a102 Mon Sep 17 00:00:00 2001 From: Maksim Zuev Date: Mon, 2 Sep 2024 12:01:54 +0200 Subject: [PATCH 08/11] Add test isDispatchNeeded throws --- .../jvm/test/ExecutorsTest.kt | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/kotlinx-coroutines-core/jvm/test/ExecutorsTest.kt b/kotlinx-coroutines-core/jvm/test/ExecutorsTest.kt index 8639f021bc..965b8fc0be 100644 --- a/kotlinx-coroutines-core/jvm/test/ExecutorsTest.kt +++ b/kotlinx-coroutines-core/jvm/test/ExecutorsTest.kt @@ -168,6 +168,33 @@ class ExecutorsTest : TestBase() { } } + @Test + fun testExceptionInIsDispatchNeeded() { + val dispatcher = object : CoroutineDispatcher() { + override fun isDispatchNeeded(context: CoroutineContext): Boolean { + expect(2) + throw TestException() + } + override fun dispatch(context: CoroutineContext, block: Runnable) = expectUnreached() + } + try { + runBlocking { + expect(1) + try { + launch(dispatcher) { + expectUnreached() + } + expectUnreached() + } catch (_: TestException) { + expect(3) + } + + } + } catch (_: TestException) { + finish(4) + } + } + private fun runTestExceptionInDispatch( totalSteps: Int, isExpectedException: (Throwable) -> Boolean, From 7d0cbc416cc48c6dafe21a00743dcc1168ed8ffd Mon Sep 17 00:00:00 2001 From: Maksim Zuev Date: Mon, 2 Sep 2024 12:18:10 +0200 Subject: [PATCH 09/11] Unwrap DispatchException --- kotlinx-coroutines-core/common/src/intrinsics/Undispatched.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kotlinx-coroutines-core/common/src/intrinsics/Undispatched.kt b/kotlinx-coroutines-core/common/src/intrinsics/Undispatched.kt index 0d2e0404fc..254182b387 100644 --- a/kotlinx-coroutines-core/common/src/intrinsics/Undispatched.kt +++ b/kotlinx-coroutines-core/common/src/intrinsics/Undispatched.kt @@ -20,7 +20,8 @@ internal fun (suspend (R) -> T).startCoroutineUndispatched(receiver: R, c startCoroutineUninterceptedOrReturn(receiver, actualCompletion) } } catch (e: Throwable) { - actualCompletion.resumeWithException(e) + val reportException = if (e is DispatchException) e.cause else e + actualCompletion.resumeWithException(reportException) return } if (value !== COROUTINE_SUSPENDED) { From df5a6fb66b7d23927ab632cc4833be0251a48bd0 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy <52952525+dkhalanskyjb@users.noreply.github.com> Date: Thu, 26 Sep 2024 11:30:58 +0200 Subject: [PATCH 10/11] Remove a redundant `finally` --- .../common/src/internal/DispatchedTask.kt | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt b/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt index 61e5902bab..cd47cf2abb 100644 --- a/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt +++ b/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt @@ -76,8 +76,6 @@ internal abstract class DispatchedTask internal constructor( final override fun run() { assert { resumeMode != MODE_UNINITIALIZED } // should have been set before dispatching - var fatalException: Throwable? = null - var dispatchException: DispatchException? = null try { val delegate = delegate as DispatchedContinuation val continuation = delegate.continuation @@ -104,13 +102,9 @@ internal abstract class DispatchedTask internal constructor( } } } catch (e: DispatchException) { - dispatchException = e + handleCoroutineException(delegate.context, e.cause) } catch (e: Throwable) { - // This instead of runCatching to have nicer stacktrace and debug experience - fatalException = e - } finally { - fatalException?.let { handleFatalException(it) } - dispatchException?.let { handleCoroutineException(delegate.context, it.cause) } + handleFatalException(e) } } From 4acd8cffedf03c47f7685b9f7d899c312171cba0 Mon Sep 17 00:00:00 2001 From: Maksim Zuev Date: Mon, 21 Oct 2024 14:07:53 +0200 Subject: [PATCH 11/11] Update kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt Co-authored-by: Vsevolod Tolstopyatov --- .../common/src/internal/DispatchedTask.kt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt b/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt index cd47cf2abb..ad5fed1205 100644 --- a/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt +++ b/kotlinx-coroutines-core/common/src/internal/DispatchedTask.kt @@ -205,7 +205,10 @@ internal inline fun Continuation<*>.resumeWithStackTrace(exception: Throwable) { } /** - * This exception holds an exception raised in [CoroutineDispatcher.dispatch] method + * This exception holds an exception raised in [CoroutineDispatcher.dispatch] method. + * When dispatcher methods fail unexpectedly, it is likely a user-induced programmatic bug, + * such as calling `executor.close()` prematurely. To avoid reporting such exceptions as fatal errors, + * we handle them with a separate code path. See also #4091. * * @see safeDispatch */