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

Rollback #2972, but leave a compatibility option with 1.6.0 #3131

Merged
merged 1 commit into from Jan 17, 2022

Conversation

qwwdfsad
Copy link
Member

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

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
@slavonnet
Copy link
Contributor

  1. This is critical issue.
  2. . Why don't you want to set the maximum parking time to 1-5 seconds and recheck the event loop? Yes, ideally it should be an unpark, but why not recheck the status and show the warning with additional information?
  3. Why not limit the blocking time of the main thread to the size of the frame time or maximum delay and not unblock the thread with the completion of the task in an exception?
  4. Why not add support for nested rubBlocking by adding it to an already existing and running coroutine as a child task? Is it enough to keep the link locally in the treadlocal?
  5. CoroutineScheduler has MAX_POOL_SIZE over 2M. At the same time, CoroutineScheduler.toString has an enumeration of index in "1 until workers.length". When debugging, they often work. Why not increase the pool by 2? It makes sense ? The costs are not significant compared to the cost of creating one thread.
  6. afterCompletion in JobSupport and BlockingCoroutine works only on successful completion, judging by the code. And if an exception or simply destroy another thread, then the blocked thread will be blocked for an infinite time?

@qwwdfsad qwwdfsad merged commit 9169d09 into develop Jan 17, 2022
@qwwdfsad qwwdfsad deleted the rollback-default-delay branch January 17, 2022 11:50
yorickhenning pushed a commit to yorickhenning/kotlinx.coroutines that referenced this pull request 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
dee-tree pushed a commit to dee-tree/kotlinx.coroutines that referenced this pull request Jul 21, 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
pablobaxter pushed a commit to pablobaxter/kotlinx.coroutines that referenced this pull request Sep 14, 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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants