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

Mark the coroutine started with UNDISPATCHED as running #4077

Merged
merged 3 commits into from Apr 3, 2024

Conversation

dkhalanskyjb
Copy link
Collaborator

This solves the issue of a coroutine started with CoroutineStart.UNDISPATCHED not being in the list of running coroutines when its code is, in fact, being executed.

It is still the case that several coroutines can be running on the same thread. For example, trying to run-to-cursor from "A" to "B" will actually enter the "B" in the outer coroutine in the following example:

suspend fun outer() {
  // run from here
  launch(start = CoroutineStart.UNDISPATCHED) {
    println("A")
    delay(1.seconds)
    target()
  }
  target()
}

suspend fun target() {
  println("B")
}

It's questionable whether this is incorrect; in any case, this is still less incorrect than what we have now, where the "B" in the inner coroutine would never get entered.

Fixes #4058

@@ -9,9 +9,12 @@ import kotlin.coroutines.intrinsics.*
* Use this function to restart a coroutine directly from inside of [suspendCoroutine],
* when the code is already in the context of this coroutine.
* It does not use [ContinuationInterceptor] and does not update the context of the current thread.
*
* Used only for tests.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably worth getting rid of this overload and use the one below, not a big fan of having test-only code in prod. Up to you

Before this change, if the `CoroutineContext` returned from
`probeCoroutineCreated` was different from the one passed to it,
the original one would be used until the first suspension.
@dkhalanskyjb dkhalanskyjb merged commit 7bdc901 into develop Apr 3, 2024
1 check passed
@dkhalanskyjb dkhalanskyjb deleted the dk-undispatched-is-running branch April 3, 2024 11:34
Corje pushed a commit that referenced this pull request May 10, 2024
Fixes #4058

Additional small fix: the coroutine context injected by `probeCoroutineCreated` is used for `CoroutineStart.UNDISPATCHED` even before the first suspension.
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