Skip to content

Dispatchers.setMain does not implement a correct Dispatchers.Main.immediate #3298

Open
@dkhalanskyjb

Description

@dkhalanskyjb

https://kotlin.github.io/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines/-main-coroutine-dispatcher/immediate.html states that, if Dispatchers.Main.immediate is supported, it behaves like Dispatchers.Unconfined when already in the correct execution context; otherwise, it should throw UnsupportedOperationException. However, this is not the case for Dispatchers.setMain, which will just use the dispatcher itself for Dispatchers.Main.immediate, without preventing stack overflows or eager execution of coroutines.

This causes real problems in the test code. The main example is viewModelScope, a CoroutineScope that corresponds to a ViewScope, Android-specific storage of data for use in UI. Because of being UI-related, viewModelScope uses the UI thread by default, and to avoid unnecessary dispatches when working with a ViewModel from the main thread, it uses Dispatchers.Main.immediate.

Code that tests functions using viewModelScope is typically structured in a way that expects the dispatches to happen eagerly—corresponding to real-world behavior in the typical case—which necessitates the use of UnconfinedTestDispatcher in Dispatchers.setMain for such tests. However, this will lead to incorrect dispatching behavior for just Dispatchers.Main, which is supposed to behave like a StandardTestDispatcher.

The most straightforward idea is two-part:

1. Make StandardTestDispatcher a MainCoroutineDispatcher, confined to the thread and exposing a proper immediate.

Problems:

  • What is the test thread? If we just check the thread at the time of creation of the test dispatcher, can this can cause problems with multithreaded test runners?
  • Calls to advance*/runCurrent from other threads will hang if we implement this naively. Maybe a better idea is not to confine the execution to one thread, but to consider the thread doing the innermost advance*/runCurrent to be the "main" thread.

2. Throw UnsupportedOperationException for Dispatchers.Main.immediate for non-MainCoroutineDispatcher arguments to Dispatchers.setMain.

This is a breaking change.

  • We could lessen the issue by also making UnconfinedTestDispatcher a MainCoroutineDispatcher, but that would promote its incorrect use.
  • Maybe, instead of breaking Dispatchers.setMain, we should deprecate it in favor of something else. For example, if we end up implementing Test module shall provide ability to replace Dispatchers.IO and Dispatchers.Default #982, we could just provide something like Dispatchers.mock(main, default) that would also fix the issue with Main.immediate.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions