Skip to content

Commit

Permalink
Do not initialize non-mocked Dispatchers.Main unnecessarily (#4301)
Browse files Browse the repository at this point in the history
Before this change, the following could happen on the JVM:
* `kotlinx.coroutines.test` accesses `Dispatchers.Main` before
  `setMain` is called.
* `Dispatchers.Main` attempts to initialize *some other* Main
  dispatcher in addition to the `kotlinx-coroutines-test` Main
  dispatcher, if there is one.
* If the `kotlinx-coroutines-android` artifact is present + its
  R8 rules are used to minify the tests, it's illegal to
  attempt to fail to create a `Main` dispatcher.
  `SUPPORT_MISSING = false` ensures that attempting to create a
  Main dispatcher fails immediately, in the call frame that
  attempts the creation, not waiting for `Dispatchers.Main` to
  be meaningfully used.

In total, `kotlinx-coroutines-test` + `kotlinx-coroutines-android`
+ R8 minification of tests leads to some patterns of
`kotlinx-coroutines-test` usage to crash.

It turns out, though, that the problem originally reported was
simply because of incorrect `kotlinx-coroutines-test` usage.
The error message got improved to guide the programmer in
such scenarios better.

Fixes #4297
  • Loading branch information
dkhalanskyjb authored Dec 19, 2024
1 parent f8c0304 commit 8f83057
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 13 deletions.
23 changes: 14 additions & 9 deletions kotlinx-coroutines-test/common/src/internal/TestMainDispatcher.kt
Original file line number Diff line number Diff line change
Expand Up @@ -9,31 +9,36 @@ import kotlin.coroutines.*
* The testable main dispatcher used by kotlinx-coroutines-test.
* It is a [MainCoroutineDispatcher] that delegates all actions to a settable delegate.
*/
internal class TestMainDispatcher(delegate: CoroutineDispatcher):
internal class TestMainDispatcher(createInnerMain: () -> CoroutineDispatcher):
MainCoroutineDispatcher(),
Delay
{
private val mainDispatcher = delegate
private var delegate = NonConcurrentlyModifiable(mainDispatcher, "Dispatchers.Main")
internal constructor(delegate: CoroutineDispatcher): this({ delegate })

private val mainDispatcher by lazy(createInnerMain)
private var delegate = NonConcurrentlyModifiable<CoroutineDispatcher?>(null, "Dispatchers.Main")

private val dispatcher
get() = delegate.value ?: mainDispatcher

private val delay
get() = delegate.value as? Delay ?: defaultDelay
get() = dispatcher as? Delay ?: defaultDelay

override val immediate: MainCoroutineDispatcher
get() = (delegate.value as? MainCoroutineDispatcher)?.immediate ?: this
get() = (dispatcher as? MainCoroutineDispatcher)?.immediate ?: this

override fun dispatch(context: CoroutineContext, block: Runnable) = delegate.value.dispatch(context, block)
override fun dispatch(context: CoroutineContext, block: Runnable) = dispatcher.dispatch(context, block)

override fun isDispatchNeeded(context: CoroutineContext): Boolean = delegate.value.isDispatchNeeded(context)
override fun isDispatchNeeded(context: CoroutineContext): Boolean = dispatcher.isDispatchNeeded(context)

override fun dispatchYield(context: CoroutineContext, block: Runnable) = delegate.value.dispatchYield(context, block)
override fun dispatchYield(context: CoroutineContext, block: Runnable) = dispatcher.dispatchYield(context, block)

fun setDispatcher(dispatcher: CoroutineDispatcher) {
delegate.value = dispatcher
}

fun resetDispatcher() {
delegate.value = mainDispatcher
delegate.value = null
}

override fun scheduleResumeAfterDelay(timeMillis: Long, continuation: CancellableContinuation<Unit>) =
Expand Down
31 changes: 28 additions & 3 deletions kotlinx-coroutines-test/jvm/src/internal/TestMainDispatcherJvm.kt
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,23 @@ internal class TestMainDispatcherFactory : MainDispatcherFactory {
override fun createDispatcher(allFactories: List<MainDispatcherFactory>): MainCoroutineDispatcher {
val otherFactories = allFactories.filter { it !== this }
val secondBestFactory = otherFactories.maxByOrNull { it.loadPriority } ?: MissingMainCoroutineDispatcherFactory
val dispatcher = secondBestFactory.tryCreateDispatcher(otherFactories)
return TestMainDispatcher(dispatcher)
/* Do not immediately create the alternative dispatcher, as with `SUPPORT_MISSING` set to `false`,
it will throw an exception. Instead, create it lazily. */
return TestMainDispatcher({
val dispatcher = try {
secondBestFactory.tryCreateDispatcher(otherFactories)
} catch (e: Throwable) {
reportMissingMainCoroutineDispatcher(e)
}
if (dispatcher.isMissing()) {
reportMissingMainCoroutineDispatcher(runCatching {
// attempt to dispatch something to the missing dispatcher to trigger the exception.
dispatcher.dispatch(dispatcher, Runnable { })
}.exceptionOrNull()) // can not be null, but it does not matter.
} else {
dispatcher
}
})
}

/**
Expand All @@ -24,4 +39,14 @@ internal actual fun Dispatchers.getTestMainDispatcher(): TestMainDispatcher {
val mainDispatcher = Main
require(mainDispatcher is TestMainDispatcher) { "TestMainDispatcher is not set as main dispatcher, have $mainDispatcher instead." }
return mainDispatcher
}
}

private fun reportMissingMainCoroutineDispatcher(e: Throwable? = null): Nothing {
throw IllegalStateException(
"Dispatchers.Main was accessed when the platform dispatcher was absent " +
"and the test dispatcher was unset. Please make sure that Dispatchers.setMain() is called " +
"before accessing Dispatchers.Main and that Dispatchers.Main is not accessed after " +
"Dispatchers.resetMain().",
e
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ open class FirstMockedMainTest : TestBase() {
component.launchSomething()
throw component.caughtException
} catch (e: IllegalStateException) {
assertTrue(e.message!!.contains("Dispatchers.setMain from kotlinx-coroutines-test"))
assertTrue(e.message!!.contains("Dispatchers.setMain"))
}
}
}

0 comments on commit 8f83057

Please sign in to comment.