vert-x3 / vertx-lang-kotlin

Vert.x for Kotlin
Apache License 2.0
296 stars 68 forks source link

Starting a coroutine on a vert.x worker thread runs it on the same thread but should hand it over to the event loop thread #217

Closed bbyk closed 1 year ago

bbyk commented 2 years ago

Questions

Starting a coroutine on a vert.x worker thread ends up running it inline, that is on the very same thread, instead of the event loop thread. This seems like a bug because it contradicts the KDoc on Context.dispatcher()

Version

vert.x 4.1.5, kotlin 1.5.31, coroutines 1.5.2

Context

VertxCoroutineExecutor

private class VertxCoroutineExecutor(val vertxContext: Context) : AbstractExecutorService(), ScheduledExecutorService {

  override fun execute(command: Runnable) {
    if (Vertx.currentContext() != vertxContext) {
      vertxContext.runOnContext { command.run() }
    } else {
      command.run()
    }
  }

I suspect the code might be lacking a check for isOnWorkerThread e.g. the if-statement should look like:

if (Vertx.currentContext() != vertxContext || Context.isOnWorkerThread()) {
      vertxContext.runOnContext { command.run() }
image

Do you have a reproducer?

            awaitBlocking {
                launch {
                    println(Thread.currentThread().name)
                }
            }

Prints

vert.x-worker-thread-0 @coroutine#23
tsegismont commented 1 year ago

I think the behavior is right but the doc is wrong.

The purpose of io.vertx.kotlin.coroutines.VertxCoroutineKt#dispatcher(io.vertx.core.Context) is to launch coroutines on the corresponding context. And inside awaitBlocking, we're still on the same Vert.x context (but on a worker thread).

tsegismont commented 1 year ago

If that's fine with you @vietj , I'll update the doc

tsegismont commented 1 year ago

In fact, this was indeed a bug, which has been fixed by https://github.com/vert-x3/vertx-lang-kotlin/pull/237