vert-x3 / vertx-lang-kotlin

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

Provide a way to wrap coroutines as Vert.x Futures #136

Closed bergmanngabor closed 1 year ago

bergmanngabor commented 5 years ago

The module vertx-lang-kotlin-coroutines has nice support for Kotlin coroutines calling/awaiting Vert.x Futures, but not the other way around. A coroutine -> Future wrapper would allow legacy codebases that still primarily operate on Vert.x Futures to gradually introduce Kotlin coroutines while still keeping a Future-based API.

bergmanngabor commented 5 years ago

This is what my colleague @abelhegedus and I have come up with:

/**
 * Wraps a coroutine as a Vertx [Future].
 *
 * Executes the block on its own embedded coroutine scope,
 *  so that all child coroutines must stop before the future completes.
 *  This way the future can be used as an asynchronous completion handler
 *  for long-running asynchronous subtasks
 */
fun <T> CoroutineScope.vertxFuture(
    context: CoroutineContext = EmptyCoroutineContext,
    start: CoroutineStart = CoroutineStart.DEFAULT,
    block: suspend CoroutineScope.() -> T
): Future<T> = Future.future<T>().also { future ->
    launch(context, start) {
        try {
            val r = coroutineScope {
                block.invoke(this)
            }
            future.complete(r)
        } catch (t: Throwable) {
            future.fail(t)
        }
    }
}

/**
 * Starts a suspendable function as a coroutine using Vertx's thread pool as dispatcher,
 *  and returns the result as a Vertx [Future].
 *
 * Executes the block on its own embedded coroutine scope,
 *  so that all child coroutines must stop before the future completes.
 *  This way the future can be used as an asynchronous completion handler
 *  for long-running asynchronous subtasks
 */
fun <T> Vertx.coroutineAsFuture(
    context: CoroutineContext = EmptyCoroutineContext,
    start: CoroutineStart = CoroutineStart.DEFAULT,
    block: suspend CoroutineScope.() -> T
): Future<T> = GlobalScope.vertxFuture(
    context + this.dispatcher(),
    start,
    block
)

It seems to work nicely for us, but it may or may not be what the project maintainers want to include (I guess especially the coroutineScope wrapping is debatable). If you like the code, I can make a PR and include it e.g. in VertxCoroutine.kt.

gmariotti commented 5 years ago

Sounds reasonable to me, however, I know that there are going to be changes related to Vert.x Future so I think that @vietj is probably the best person to know if it makes sense to have it.

vietj commented 5 years ago

in the near future we are going to deprecate Future#complete method and likely replace it with a Promise interface. The Future interface will still exist but will be read-only and the Promise will allow to get the associated Future.

So you might want contribute this when the code has changed I think.

ShreckYe commented 3 years ago
fun <T> CoroutineScope.coroutineToFuture(
    context: CoroutineContext = EmptyCoroutineContext,
    start: CoroutineStart = CoroutineStart.DEFAULT,
    block: suspend CoroutineScope.() -> T
): Future<T> {
    val promise = Promise.promise<T>()
    launch(context, start) {
        try {
            promise.complete(block())
        } catch (t: Throwable) {
            promise.fail(t)
        }
    }
    return promise.future()
}

I coded a version using the Promise API. Is this correct?

bergmanngabor commented 3 years ago

@ShreckYe compared to your code above, the solution further up has one main difference (apart from the deprecated Future API): a coroutine scope wrapping around the actual invocation of the block. I am not an expert and am unsure whether this is a problem - we just wanted to play it as safe as possible with respect to coroutine cancellation: our line of thought was that upon the inner coroutine being cancelled, the calling context might not want itself to be cancelled, but handle the Future as a normal failed Future. I am not sure whether this is a good goal to aim for, and whether we have achieved it correctly (maybe it should have been supervisorScope() instead?). It would be great if an actual expert weighed in :)

On the other hand, for user friendliness, I think you still need the second method from our earlier post Vertx.coroutineAsFuture() so that the user can simply invoke a coroutine on their vertx instance.

ShreckYe commented 3 years ago

@bergmanngabor Thanks for your advice. I am not an expert in coroutine details either so it would be good that an actual expert weighed in here.

As for the second method, it's not really necessary in my case where I use them in a CoroutineVerticle, which is already a CoroutineScope with a coroutineContext of context.dispatcher(), which I think should be the recommended dispatcher. However, it might be necessary for other situations that I haven't encountered yet.

tsegismont commented 1 year ago

Duplicates #209