Coroutines: UI event race condition?

1

Consider the following race condition:

void onButtonClicked() {
    launch(uiEventLoop) {
        disableButton()
        doWork.await()
        enableButton()
    }
}

The problem is that disableButton() is first posted to the event loop before executed and the user could click the button again in the meantime. This means doWork could run twice in parallel what is probably unintended. The solution is to call disableButton() before launching the coroutine to prevent another click. However, this is a subtle problem which many devs would probably not notice or ignore. Furthermore, the disable/enable part would be in two different code blocks and can’t be automated using extension functions…

I wonder if it would make sense to add special dispatchers that run the first part of the coroutine in the current thread. For example, a JavaFXEventHandler dispatcher that runs the first dispatch call in the current thread (only if it is indeed the java fx thread). Or to include this behaviour on default for UI dispatchers.

Or add a helper (simplified):

class StartNow(val parent: CoroutineDispatcher) : CoroutineDispatcher() {
    private var first = true

    override fun dispatch(context: CoroutineContext, block: Runnable) {
        if (first) {
            first = false
            block.run()
        } else
            parent.dispatch(context, block)
    }
}
2

If you want disableButton() to be invoked immediately, you can either move its invocation outside of launch or add an additional parameter CoroutineStart.UNDISPATCHED to the launch. The later is good for a framework code – if you are writing your own Kotlin DSL for UIs with onButtonClicked helper function, then you should definitely do launch(uiEventLoop, CoroutineStart.UNDISPATCHED) { ... } inside of it, because inside of button click handler you know that you are already on an even loop handling a particular event.

You can read more about it and some other general advise in the Guide to UI programming with coroutines.

I would also highly recommend to consider using some higher-level architecture in your UI code that completely takes care about the aspects like disabling async action buttons, showing progress, etc for you by binding the state of the corresponding UI elements to the state of the corresponding async actions. It also helps with lifecycle, by decoupling async actions themselves (that can be long-running) from the UI (which can be destroyed and rebuilt on screen rotation, for example).

1 Like
3

Cool awesome, I missed this flag!

Edit: From the UI Guide (Blocking operations):

This is not a big problem in practice, because run is smart enough to check that the coroutine is already running in the required context and avoids overhead of dispatching coroutine to a different thread again.

Is that true? Is it equivalent to the UNDISPATCHED flag?