Skip to content

Commit 8f83057

Browse files
authored
Do not initialize non-mocked Dispatchers.Main unnecessarily (#4301)
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
1 parent f8c0304 commit 8f83057

File tree

3 files changed

+43
-13
lines changed

3 files changed

+43
-13
lines changed

kotlinx-coroutines-test/common/src/internal/TestMainDispatcher.kt

+14-9
Original file line numberDiff line numberDiff line change
@@ -9,31 +9,36 @@ import kotlin.coroutines.*
99
* The testable main dispatcher used by kotlinx-coroutines-test.
1010
* It is a [MainCoroutineDispatcher] that delegates all actions to a settable delegate.
1111
*/
12-
internal class TestMainDispatcher(delegate: CoroutineDispatcher):
12+
internal class TestMainDispatcher(createInnerMain: () -> CoroutineDispatcher):
1313
MainCoroutineDispatcher(),
1414
Delay
1515
{
16-
private val mainDispatcher = delegate
17-
private var delegate = NonConcurrentlyModifiable(mainDispatcher, "Dispatchers.Main")
16+
internal constructor(delegate: CoroutineDispatcher): this({ delegate })
17+
18+
private val mainDispatcher by lazy(createInnerMain)
19+
private var delegate = NonConcurrentlyModifiable<CoroutineDispatcher?>(null, "Dispatchers.Main")
20+
21+
private val dispatcher
22+
get() = delegate.value ?: mainDispatcher
1823

1924
private val delay
20-
get() = delegate.value as? Delay ?: defaultDelay
25+
get() = dispatcher as? Delay ?: defaultDelay
2126

2227
override val immediate: MainCoroutineDispatcher
23-
get() = (delegate.value as? MainCoroutineDispatcher)?.immediate ?: this
28+
get() = (dispatcher as? MainCoroutineDispatcher)?.immediate ?: this
2429

25-
override fun dispatch(context: CoroutineContext, block: Runnable) = delegate.value.dispatch(context, block)
30+
override fun dispatch(context: CoroutineContext, block: Runnable) = dispatcher.dispatch(context, block)
2631

27-
override fun isDispatchNeeded(context: CoroutineContext): Boolean = delegate.value.isDispatchNeeded(context)
32+
override fun isDispatchNeeded(context: CoroutineContext): Boolean = dispatcher.isDispatchNeeded(context)
2833

29-
override fun dispatchYield(context: CoroutineContext, block: Runnable) = delegate.value.dispatchYield(context, block)
34+
override fun dispatchYield(context: CoroutineContext, block: Runnable) = dispatcher.dispatchYield(context, block)
3035

3136
fun setDispatcher(dispatcher: CoroutineDispatcher) {
3237
delegate.value = dispatcher
3338
}
3439

3540
fun resetDispatcher() {
36-
delegate.value = mainDispatcher
41+
delegate.value = null
3742
}
3843

3944
override fun scheduleResumeAfterDelay(timeMillis: Long, continuation: CancellableContinuation<Unit>) =

kotlinx-coroutines-test/jvm/src/internal/TestMainDispatcherJvm.kt

+28-3
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,23 @@ internal class TestMainDispatcherFactory : MainDispatcherFactory {
88
override fun createDispatcher(allFactories: List<MainDispatcherFactory>): MainCoroutineDispatcher {
99
val otherFactories = allFactories.filter { it !== this }
1010
val secondBestFactory = otherFactories.maxByOrNull { it.loadPriority } ?: MissingMainCoroutineDispatcherFactory
11-
val dispatcher = secondBestFactory.tryCreateDispatcher(otherFactories)
12-
return TestMainDispatcher(dispatcher)
11+
/* Do not immediately create the alternative dispatcher, as with `SUPPORT_MISSING` set to `false`,
12+
it will throw an exception. Instead, create it lazily. */
13+
return TestMainDispatcher({
14+
val dispatcher = try {
15+
secondBestFactory.tryCreateDispatcher(otherFactories)
16+
} catch (e: Throwable) {
17+
reportMissingMainCoroutineDispatcher(e)
18+
}
19+
if (dispatcher.isMissing()) {
20+
reportMissingMainCoroutineDispatcher(runCatching {
21+
// attempt to dispatch something to the missing dispatcher to trigger the exception.
22+
dispatcher.dispatch(dispatcher, Runnable { })
23+
}.exceptionOrNull()) // can not be null, but it does not matter.
24+
} else {
25+
dispatcher
26+
}
27+
})
1328
}
1429

1530
/**
@@ -24,4 +39,14 @@ internal actual fun Dispatchers.getTestMainDispatcher(): TestMainDispatcher {
2439
val mainDispatcher = Main
2540
require(mainDispatcher is TestMainDispatcher) { "TestMainDispatcher is not set as main dispatcher, have $mainDispatcher instead." }
2641
return mainDispatcher
27-
}
42+
}
43+
44+
private fun reportMissingMainCoroutineDispatcher(e: Throwable? = null): Nothing {
45+
throw IllegalStateException(
46+
"Dispatchers.Main was accessed when the platform dispatcher was absent " +
47+
"and the test dispatcher was unset. Please make sure that Dispatchers.setMain() is called " +
48+
"before accessing Dispatchers.Main and that Dispatchers.Main is not accessed after " +
49+
"Dispatchers.resetMain().",
50+
e
51+
)
52+
}

ui/kotlinx-coroutines-android/android-unit-tests/test/ordered/tests/FirstMockedMainTest.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ open class FirstMockedMainTest : TestBase() {
3535
component.launchSomething()
3636
throw component.caughtException
3737
} catch (e: IllegalStateException) {
38-
assertTrue(e.message!!.contains("Dispatchers.setMain from kotlinx-coroutines-test"))
38+
assertTrue(e.message!!.contains("Dispatchers.setMain"))
3939
}
4040
}
4141
}

0 commit comments

Comments
 (0)