Hello, I’m trying to figure out what is the best p...

# coroutines

[dephinera]

Hello,
I’m trying to figure out what is the best practice to specify a Dispatcher.
Let’s say we have a typical situation where a ViewModel wants to load some data that is to be displayed to the user. Then a coroutine will be launched on the Main Dispatcher and the result will be handled there. However loading the data requires some IO. Here comes the question: who has to declare which Dispatcher should the IO operation be executed on? Should the VM wrap the hypothetical suspend fun loadData() with withContext(<http://Dispatchers.IO|Dispatchers.IO>) , or should the loadData() function itself use withContext, as the function is the one who knows there’s an actual IO. loadData() might require some computational work instead of IO, in which case Dispatchers.Default would be preferred (to make thinks more clear, I’ll leave examples in a thread). Is my understanding correct?

The only rule I can remember from the coroutines codelab from JB is that if the function takes a callback, you should call that callback outside of any withContext blocks, as the consumer declares where the result should be handled.

[dephinera]

• Example 1 - the correct one IMO

class MyVM(private val repository: Repository) {
    fun init() {
        viewModelScope.launch(Dispatchers.Main.Immediate) {
            val dataToBeDisplayed = repository.loadData()
            handleResult(dataToBeDisplayed)
        }
    }
}
class Repository {
    suspend fun loadData(): Result {
        return withContext(<http://Dispatchers.IO|Dispatchers.IO>) {
            doIO()
        }
    }
}

• Example 2

class MyVM(private val repository: Repository) {
    fun init() {
        viewModelScope.launch(Dispatchers.Main.Immediate) {
            val dataToBeDisplayed = withContext(<http://Dispatchers.IO|Dispatchers.IO>) {
                repository.loadData()
            }
            handleResult(dataToBeDisplayed)
        }
    }
}
class Repository {
    suspend fun loadData(): Result {
        return doIO()
    }
}

[Sam]

✅ by convention, a suspend function should not call blocking functions, and should be safe to call on any dispatcher. The caller shouldn’t need to know the implementation details of the function in order to call it safely. So I agree, your first example is the correct choice 👍

[dephinera]

Thank you for confirming this.

[Sam]

Here’s a reference: https://elizarov.medium.com/blocking-threads-suspending-coroutines-d33e11bf4761 (in the section on “Suspending convention”)

[stojan]

you can also drop the Dispatchers.Main.Immediate from the launch as viewModelScope runs on Dispatchers.Main.Immediate by default

and yes, first option is the recommended one... functions are responsible for running on the correct scope:

https://developer.android.com/kotlin/coroutines/coroutines-best-practices#main-safe

[gildor]

suspend function should not call blocking functions

Suspend function can call blocking function if wrap it correctly (with appropriate dispatcher and handle cancellation correctly when it’s possilbe), the convention is that suspend function shouldn’t be blocking by itself

[Sam]

Good clarification, my wording was incorrect 👍

[Ryan Smith]

Suspend function can call blocking function if wrap it correctly (with appropriate dispatcher and handle cancellation correctly when it’s possilbe)

As an example, then, if I wanted to do something like file I/O what would be the correct way to manage it?

I have an example here, if it helps:

suspend fun <T> serializeIt(thing: T) {
    withContext(<http://Dispatchers.IO|Dispatchers.IO>) {
        val file = File("out.json")
        file.outputStream().use {
            // Json from kotlinx.serialization
            Json.encodeToStream(thing, it)
        }
    }
}

[gildor]

Yes. There are a few pitfalls, but it’s not so easy to avoid them every time
This particular function will not be cancelled when coroutine is cancelled, though result will be discarded
But it’s not so a huge a problem on practice (at least usually), so personally I usually just ignore it

The perfect solution would be to close stream if scope is cancelled, but don’t think that it supported be encodeToStream, you can emulate it with something like:

withContext(…) {
   use {
      coroutineContext.job.invokeOnCompletion { it.close() }
   }
}

[Ryan Smith]

In the example you provided is it possible that due to cancellation the stream is closed in the middle of a write causing the output file to be "corrupted"? I suppose when the alternative is leaking the file a little corrupted data isn't the worst. Maybe I've got it wrong, and invokeOnCompletion will shut down everything neatly?

[gildor]

Nope, it wouldn’t shutdown neatly, indeed, you will have corrupted stream.
Depends what you want from cancellation.
It actually the problem of original code too, so you need try/catch with clean up if operation failed (for example cases like not enough disk space etc)

[Ryan Smith]

Is the resource safety provided by use not enough to clean up the stream in the event of cancellation? Does cancellation not result in a CancellationException being thrown?

[gildor]

No, it’s not enough, use just make sure that stream is closed and resource is not leaked

[gildor]

Does cancellation not result in a CancellationException being thrown

It would, but your code doesn’t call any suspend function, so cancellation will be thrown when withContext block is finihsed, it will be propagated on top

[gildor]

@Ryan Smith You can check it yourself:

val file = createTempFile()
try {
    file.outputStream().use {
        it.write("{".toByteArray())
        throw CancellationException()
        it.write("}".toByteArray())
    }

} catch (e: CancellationException) {
    println("Resulting file content: ${file.readText()}")
}
println("Completed")

[gildor]

Result will be:

Resulting file content: {
Completed

[gildor]

So to fix you need to clean it up, and it not only related to coroutines:

val file = createTempFile()
try {
    file.outputStream().use {
        it.write("{".toByteArray())
        throw CancellationException()
        it.write("}".toByteArray())
    }

} catch (e: Exception) {
     try {
         file.delete()
     } catch (e: Exception) {
         println("Not able to clean up")
     }
    println("Resulting file exists: ${file.exists()}")
}
println("Completed")

// Out:
// Resulting file exists: false
// Completed

[gildor]

I believe NIO has this feature out of the box to allow delete file on close of the stream, but NIO is not available on Android (at least until recent version of desugar lib)

[Ryan Smith]

No worries there, I'm working on Compose Desktop so I think all the NIO API as of Java 15 is available to me with no trouble.

This raises a broader question for me w.r.t. cancellation. Since use and the block I pass to it are blocking, how does cancellation affect them? Is the enclosing scope not capable of cleaning up until my use block returns normally (or exceptionally)?

From our discussion it sounds like what happens is the enclosing scope and all of it's children are canceled, leaving use unaware that something exceptional has occurred. But what then? Does it run to completion, resulting in a cancellation exception when it returns? If so it seems that while the result of use would be lost, the file output stream wouldn't leak since use would return.

[gildor]

Yes, everything what you mention is correct

[gildor]

without interrupting of writing, stream will continue and will cause leak of resource until operation is completed

[Ryan Smith]

Okay cool, I think I'm understanding a little bit better

[gildor]

Coroutines also has runInterruptible - https://kotlinlang.org/api/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines/run-interruptible.html
So it allows currently interrupt thread with InterruptedException, but it works only with blocking code which supports it, but honestly looking into encodeToStream nothing looks interruptable for me. but maybe I’m missing something

[Ryan Smith]

I agree with that assessment, all of the Json.encodeToStream logic seems synchronous from what I can tell

[gildor]

it’s not a problem that it syncronous, Thread.sleep() also syncrnous, but it supports Thread.interrupted(), so can be cancelled/interrupted and allows to handle it correctly with runInterruptible

[Ryan Smith]

Right, my bad definitely a poor choice of words

[gildor]

No problem, it’s really not so straight forward, otherwise it looks that I hand on every word in this thread 🙈

[Ryan Smith]

I appreciate the discussion, thanks for being so patient