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

Coroutine on EventLoop dispatcher fails to yield to a task which is resuming after a delay #4134

Closed
roomscape opened this issue May 27, 2024 · 5 comments
Labels

Comments

@roomscape
Copy link

roomscape commented May 27, 2024

fun main() = runBlocking {
  launch {
    delay(1)
    println("This should be first")
  }
  Thread.sleep(1000)
  yield()
  println("This should be second")
}

Actual behaviour

The code outputs:

This should be second
This should be first

The call to yield() resumes immediately, instead of yielding control to the other coroutine.

This seems to be an issue with the EventLoop dispatcher used by runBlocking. It maintains a separate queue for delayed continuations. On each iteration of the event loop, tasks whose delay has expired are moved from the delay queue to the main event queue. During a call to yield(), this takes place after the yield() function has added its own continuation to the event queue. That means a yield() call will always fail to yield if the only other continuations waiting to resume are ones whose delay has expired since the last dispatch.

Expected behaviour

Output should be:

This should be first
This should be second

On reaching the call to yield(), the launch job is already eligible to resume. 1 second has elapsed, which is much longer than the 1ms required delay. The dispatcher should prefer to dispatch the launch job, instead of resuming the same coroutine that called yield().

Environment

Reproduced using Kotlin 1.9 with coroutines 1.8.1, on JVM 1.8.

This issue was first reported by a user on Slack: https://kotlinlang.slack.com/archives/C0B8MA7FA/p1716495410746749

@dkhalanskyjb
Copy link
Collaborator

Here's the sequence of events:

  • A new coroutine is launched.
  • Thread.sleep.
  • yield.
  • Now, the thread can start the coroutine that was launch-ed.
  • delay is called, suspending the new coroutine.
  • While the new coroutine is suspended, the old one prints the text.
  • The new coroutine wakes up and also prints its text.

That said, this example should work the way you describe:

import kotlinx.coroutines.*

fun main() = runBlocking {
  launch(start = CoroutineStart.UNDISPATCHED) {
    delay(1)
    println("This should be first")
  }
  Thread.sleep(1000)
  yield()
  println("This should be second")
}

Yet it, too, doesn't.

So, it looks like there is a real bug here, but at the same time, the example you've submitted is working as intended.

@roomscape
Copy link
Author

Oops! I guess when I was simplifying my reproducer code, I accidentally simplified it too much. Thank you for fixing my mistake. Your explanation makes complete sense and I can now see why the code I shared is actually not demonstrating the bug 🤦.

@qwwdfsad
Copy link
Collaborator

The reason behind that behaviour is that internally, there are two event loops -- one is the regular tasks queue, and the other one is the timed tasks queue.

When a timed event fires (meaning it becomes eligible for execution), it is moved to the tasks queue first (which is FIFO), and then the task queue is processed.

In theory, we can workaround that by nano-timestamping each event and providing some semi-strict ordering, or by checking timed queue on each regular enqueue, but we should be really careful about the consequences

@dkhalanskyjb
Copy link
Collaborator

dkhalanskyjb commented May 27, 2024

Actually, @roomscape deserves credit for having already identified this: the same analysis is present in the initial message as well. Thanks for the well-researched issue report!

@qwwdfsad
Copy link
Collaborator

I took a shortcut and looked at the minified example and expected/actual results first, and then immediately started to think about the reasons 😅
Thanks, @roomscape, indeed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants