Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not initialize non-mocked Dispatchers.Main unnecessarily #4301

Merged
merged 2 commits into from
Dec 19, 2024

Conversation

dkhalanskyjb
Copy link
Collaborator

@dkhalanskyjb dkhalanskyjb commented Dec 16, 2024

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.

Fixes #4297

@dkhalanskyjb dkhalanskyjb requested review from qwwdfsad and removed request for qwwdfsad December 16, 2024 12:41
@dkhalanskyjb dkhalanskyjb marked this pull request as draft December 16, 2024 13:06
@dkhalanskyjb dkhalanskyjb force-pushed the dk-fix-extra-work-in-Dispatchers.setMain branch from 1ac3943 to 91eae91 Compare December 16, 2024 13:09
@dkhalanskyjb dkhalanskyjb marked this pull request as ready for review December 16, 2024 13:09
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.

Fixes #4297
@dkhalanskyjb dkhalanskyjb force-pushed the dk-fix-extra-work-in-Dispatchers.setMain branch from 91eae91 to efd4264 Compare December 16, 2024 13:13
@dkhalanskyjb dkhalanskyjb force-pushed the dk-fix-extra-work-in-Dispatchers.setMain branch from 3082337 to 06df055 Compare December 17, 2024 14:03
@dkhalanskyjb dkhalanskyjb merged commit 8f83057 into develop Dec 19, 2024
1 check passed
@dkhalanskyjb dkhalanskyjb deleted the dk-fix-extra-work-in-Dispatchers.setMain branch December 19, 2024 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants