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

Default delay behavior on Android creates additional threads when using external dispatchers #2972

Open
adamp opened this issue Oct 11, 2021 · 5 comments

Comments

@adamp
Copy link

@adamp adamp commented Oct 11, 2021

When delay is called using an externally-defined dispatcher that does not implement the internal Delay interface, it uses a default fallback implementation that can cause an additional thread to be created. Some Android apps strictly audit threads created by their library dependencies and have raised this as an issue when using Jetpack Compose, which defines its own UI dispatcher and both promotes using the delay API and uses it internally.

On Android, apps always have a main thread running an event loop that could be used in place of creating an additional thread to time the delay, which could make Dispatchers.Main a suitable choice for performing the default delay on Android when a compatible/known dispatcher is not found.

@elizarov
Copy link
Member

@elizarov elizarov commented Oct 11, 2021

Indeed, we could design & implement a pluggable implementation for a default delay mechanism that kotlinx-coroutines-android module could implement on top of Android's main thread.

qwwdfsad added a commit that referenced this issue Oct 11, 2021
It reduces the number of redundant threads in the system and makes time source predictable across Android/JavaFx applications

Fixes #2972
qwwdfsad added a commit that referenced this issue Nov 17, 2021
It reduces the number of redundant threads in the system and makes time source predictable across Android/JavaFx applications

Fixes #2972
qwwdfsad added a commit that referenced this issue Nov 17, 2021
It reduces the number of redundant threads in the system and makes time source predictable across Android/JavaFx applications

Fixes #2972
@qwwdfsad
Copy link
Member

@qwwdfsad qwwdfsad commented Nov 17, 2021

To opt-out from this behaviour, a temporary system property kotlinx.coroutines.main.delay can be set to false

@tanmatra
Copy link

@tanmatra tanmatra commented Nov 24, 2021

Shouldn't variables declarations be swapped? See #3044.
Currently DefaultDelay is assigned before defaultMainDelayOptIn is assigned (and is initially false).

internal actual val DefaultDelay: Delay = initializeDefaultDelay()

private val defaultMainDelayOptIn = systemProp("kotlinx.coroutines.main.delay", true)

private fun initializeDefaultDelay(): Delay {
    // Opt-out flag
    if (!defaultMainDelayOptIn) return DefaultExecutor
    ...
}

@qwwdfsad
Copy link
Member

@qwwdfsad qwwdfsad commented Nov 24, 2021

Right, this is the bug with the cyclic initialization

qwwdfsad added a commit that referenced this issue Jan 11, 2022
The approach from 1.6.0 has proven itself as unstable and multiple hard-to-understand bugs have been reported:

* JavaFx timer doesn't really work outside the main thread
* The frequent initialization pattern "runBlocking { doSomethingThatMayCallDelay() }" used on the main thread during startup now silently deadlocks
* The latter issue was reported both by Android and internal JB Compose users
* The provided workaround with system property completely switches off the desired behaviour that e.g. Compose may rely on, potentially introducing new sources of invalid behaviour

The original benefits does not outweigh these pitfalls, so the decision is to revert this changes in the minor release

Fixes #3113
Fixes #3106
qwwdfsad added a commit that referenced this issue Jan 17, 2022
The approach from 1.6.0 has proven itself as unstable and multiple hard-to-understand bugs have been reported:

* JavaFx timer doesn't really work outside the main thread
* The frequent initialization pattern "runBlocking { doSomethingThatMayCallDelay() }" used on the main thread during startup now silently deadlocks
* The latter issue was reported both by Android and internal JB Compose users
* The provided workaround with system property completely switches off the desired behaviour that e.g. Compose may rely on, potentially introducing new sources of invalid behaviour

The original benefits does not outweigh these pitfalls, so the decision is to revert this changes in the minor release

Fixes #3113
Fixes #3106
@qwwdfsad qwwdfsad reopened this Jan 17, 2022
@qwwdfsad
Copy link
Member

@qwwdfsad qwwdfsad commented Jan 17, 2022

We decided to revert this change in 1.6.1, see the rationale here: #3131

In the meantime, we have to find another solution for this problem

@qwwdfsad qwwdfsad removed the For 1.6 label Jan 17, 2022
yorickhenning pushed a commit to yorickhenning/kotlinx.coroutines that referenced this issue Jan 28, 2022
…#2974)

* Use Dispatchers.Main as default delay source where applicable

It reduces the number of redundant threads in the system and makes time source predictable across Android/JavaFx applications

Fixes Kotlin#2972
yorickhenning pushed a commit to yorickhenning/kotlinx.coroutines that referenced this issue Jan 28, 2022
…tlin#3131)

The approach from 1.6.0 has proven itself as unstable and multiple hard-to-understand bugs have been reported:

* JavaFx timer doesn't really work outside the main thread
* The frequent initialization pattern "runBlocking { doSomethingThatMayCallDelay() }" used on the main thread during startup now silently deadlocks
* The latter issue was reported both by Android and internal JB Compose users
* The provided workaround with system property completely switches off the desired behaviour that e.g. Compose may rely on, potentially introducing new sources of invalid behaviour

The original benefits does not outweigh these pitfalls, so the decision is to revert this changes in the minor release

Fixes Kotlin#3113
Fixes Kotlin#3106
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants