Skip to content

Commit

Permalink
Fail eagerly during exceptions in isDispatchNeeded (#2733)
Browse files Browse the repository at this point in the history
That helps to pro-actively catch cases like #2717 and to report such exception in even more robust manner
  • Loading branch information
qwwdfsad committed May 31, 2021
1 parent 937180f commit 119e443
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 6 deletions.
15 changes: 14 additions & 1 deletion kotlinx-coroutines-core/common/src/intrinsics/Cancellable.kt
Expand Up @@ -49,6 +49,19 @@ private inline fun runSafely(completion: Continuation<*>, block: () -> Unit) {
try {
block()
} catch (e: Throwable) {
completion.resumeWith(Result.failure(e))
dispatcherFailure(completion, e)
}
}

private fun dispatcherFailure(completion: Continuation<*>, e: Throwable) {
/*
* This method is invoked when we failed to start a coroutine due to the throwing
* dispatcher implementation or missing Dispatchers.Main.
* This situation is not recoverable, so we are trying to deliver the exception by all means:
* 1) Resume the coroutine with an exception, so it won't prevent its parent from completion
* 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
}
12 changes: 11 additions & 1 deletion kotlinx-coroutines-core/jvm/test/FailFastOnStartTest.kt
Expand Up @@ -70,8 +70,18 @@ class FailFastOnStartTest : TestBase() {
val actor = actor<Int>(Dispatchers.Main, start = CoroutineStart.LAZY) { fail() }
actor.send(1)
}

private fun mainException(e: Throwable): Boolean {
return e is IllegalStateException && e.message?.contains("Module with the Main dispatcher is missing") ?: false
}

@Test
fun testProduceNonChild() = runTest(expected = ::mainException) {
produce<Int>(Job() + Dispatchers.Main) { fail() }
}

@Test
fun testAsyncNonChild() = runTest(expected = ::mainException) {
async<Int>(Job() + Dispatchers.Main) { fail() }
}
}
14 changes: 10 additions & 4 deletions kotlinx-coroutines-core/jvm/test/FailingCoroutinesMachineryTest.kt
Expand Up @@ -33,7 +33,7 @@ class FailingCoroutinesMachineryTest(

private var caught: Throwable? = null
private val latch = CountDownLatch(1)
private var exceptionHandler = CoroutineExceptionHandler { _, t -> caught = t;latch.countDown() }
private var exceptionHandler = CoroutineExceptionHandler { _, t -> caught = t; latch.countDown() }
private val lazyOuterDispatcher = lazy { newFixedThreadPoolContext(1, "") }

private object FailingUpdate : ThreadContextElement<Unit> {
Expand Down Expand Up @@ -115,14 +115,20 @@ class FailingCoroutinesMachineryTest(

@Test
fun testElement() = runTest {
launch(NonCancellable + dispatcher.value + exceptionHandler + element) {}
// Top-level throwing dispatcher may rethrow an exception right here
runCatching {
launch(NonCancellable + dispatcher.value + exceptionHandler + element) {}
}
checkException()
}

@Test
fun testNestedElement() = runTest {
launch(NonCancellable + dispatcher.value + exceptionHandler) {
launch(element) { }
// Top-level throwing dispatcher may rethrow an exception right here
runCatching {
launch(NonCancellable + dispatcher.value + exceptionHandler) {
launch(element) { }
}
}
checkException()
}
Expand Down

0 comments on commit 119e443

Please sign in to comment.