I am in the process of converting a larger codebase to suspending code and thinking about best practices and what would need to be accounted for. One thing that crossed my mind is the following:
In blocking code, if one knew that several threads may access a class, it had to be made thread-safe. This was something to keep in mind but was easily solved by wrapping the critical sections in synchronized { ... }.
In Kotlin with Coroutines, the same will be achieved with myMutex.withLock { ... }, fair enough.
But there is something else with suspending methods. Here is an example. Assume takeSomeSalt and takePot are mutating methods in other classes.
Blocking code:
fun cookPotatos(potatos: List<Potato>) {
// make sure we are the only one in the kitchen
synchronized {
val salt = saltJar.takeSomeSalt()
stove.turnOn()
val pot = cupboard.takePot()
pot.add(tap.getWater())
pot.add(salt)
pot.add(potatos)
stove.put(pot)
Thread.sleep(25*60*1000)
stove.turnOff()
}
}
(Non-parallized) suspending code, looks the same. That’s good!: (By the way: who can turn this into parallelized code that is still easy to read? I tried it but failed)
suspend fun cookPotatos(potatos: List<Potato>) {
// make sure we are the only one in the kitchen
mutex.withLock {
val salt = saltJar.takeSomeSalt()
stove.turnOn()
val pot = cupboard.takePot()
pot.add(tap.getWater())
pot.add(salt)
pot.add(potatos)
stove.put(pot)
delay(25*60*1000)
stove.turnOff()
}
}
But here is the thing:
Suspending methods could be cancelled at any point during their execution, depending on the scope it had been called with. The receiver has no control over in which scope it had been called but at the same time, our Potato-Cooker-class needs to make sure that it itself is “suspend-safe”, i.e. manages its data in a way that does not lead to an inconsistent state.
This is not that obvious by just looking at the code, because they look like almost any other method. But it is something to worry about, basically for every single method that is suspending, this has to be kept in mind. I find it easy to miss, concept-wise.
Every critical section that is guarded by a mutex is critical for a reason. In this case, we might even burn down the kitchen if we leave the stove on 
. At least for my use case, basically every time I use a mutex, I also want it to not be cancellable.
What to do about it
I think it should be almost a pattern with Kotlin coroutines that every mutating method in a class that manages data by default implements a solution that makes the code non-cancellable, just as it is a pattern to first start with wrapping everything in synchronized when starting to make a class thread-safe (and optimize later).
There are two possibilities to make it non-cancellable, and I think the second is better.
- by wrapping the code in
withContext(NonCancellable) { ... }. If I understand this correctly, the downside of this would be that if for example the coroutine was started by a View which should be cleared, it cannot be garbage collected until that non-cancellable is done. So while the potatos are cooking, the View is still around and leaking memory. - by writing the method like this:
suspend fun cookPotatos(potatos: List<Potato>) = myScope.launch{ ... }.join()
Why even keep the suspend? This way, the potatos will be cooked independent of whether the View is still there, the only thing that is cancelled when the View is cleared is the join(), so same as if the method was fire and forget (like this:)…
fun cookPotatos(potatos: List<Potato>) = myScope.launch { ... }
…only that, the View will know when the potatos are done and could update its UI accordingly if it wanted. If the View doesn’t want that, but have it like a fire-and-forget, it could simply launch that method in its lifecycleScope.
So in this regard, the suspend fun f() = launch{ ... }.join() pattern is the most flexible.
Plus, it is consistent with how a mutating-non-cancellable method that returns a result would need to look like:
suspend fun cookPotatos(potatos: List<Potato>) = myScope.async{ ... }.await()
A resource for this is also what Google recommends for Android development -
So, what do you think? Would you also do it like that? Or would you prefer the NonCancellable pattern? And why?
. Is there a technical reason not to wrap the whole block in a