Is there any way to handle dropped return values f...

# coroutines

[ansman]

Is there any way to handle dropped return values from a suspending coroutine? Say I do this:

fun processImage() {
  val file = createFile()
  try {
    // Do stuff with the file
  } finally {
    file.delete
  }
}

suspend fun createFile(): File = withContext(<http://Dispatchers.IO|Dispatchers.IO>) {
  return File.createTemporaryFile("foo", "temp")
}

There is a case where createFile has finished but processImage has not yet been resumed. If during this time the job is cancelled we’ll never get a chance to clean up the file. Is there a way around this?

[uli]

Would this solve your issue?

fun processImage() {
  val file : File? = null
  try {
    file = createFile()
    // Do stuff with the file
  } finally {
    file?.delete
  }
}

[ansman]

No, it would have the same result since createFile never actually “returned” a file

[uli]

Right. The File is only returned from the withContext lambda parameter, but not yet from createFile

[uli]

That's ugly 🙂

[ansman]

I could use the unconfined dispatcher I suppose but then I’d lose the dispatcher of the parent

[uli]

Did you see that coming? Or did it fall on your feet?

[uli]

I think the clean solution is to clean up in createFile. It is so ugly because the closing brace of withContext schedules. If it were explicit like in this code:

suspend fun createFile(): File {
    var file : File? = null
    withContext(<http://Dispatchers.IO|Dispatchers.IO>) {
        file = File.createTempFile(("foo", "temp")
    }
    yield()
    return file
}

it would be obviouse that you have to fix it like this:

suspend fun createFile(): File {
    var file: File? = null
    try {
        withContext(<http://Dispatchers.IO|Dispatchers.IO>) {
            file = File.createTempFile(("foo", "temp")
        }
        yield()
    } catch (Exception e) {
        file.delete()
        throw e
    }
    return file
}

[trathschlag]

How about this:

suspend fun processImage() {
    useTemporaryFile { file ->
        // do stuff with the file
    }
}

suspend fun <R> useTemporaryFile(body: suspend (File) -> R) : R = withContext(<http://Dispatchers.IO|Dispatchers.IO>) {
    val file = createTempFile("foo", "temp")
    try {
        withContext(Dispatchers.Default) {
            body(file)
        }
    } finally {
        file.delete()
    }
}

this should always delete the file

[uli]

Almost brilliant. Except we want to go back to where we came from, not to the default dispatcher

[uli]

suspend inline fun <R> useTemporaryFile(block: suspend (File) -> R): R {
    var file: File? = null
    try {
        withContext(<http://Dispatchers.IO|Dispatchers.IO>) {
            file = createTempFile("foo", "temp")
        }
        return file?.let { block(it) } ?: throw IllegalStateException()
    } finally {
        file?.delete()
    }
}

If only `withContext`was already annotated with an appropriate contract, the compiler could probably smart cast file and we would not need the ugly `let`/`throw`

[ansman]

This was just an example to illustrate the general problem, the creation of the resource cannot be called in a blocked way

[uli]

understood. But i think @trathschlag's approach is one to remember for this general case

[uli]

while writing my code above as an exercise i almost did the same mistake again. This issue is so subtile.
The following code has the same, original problem:

// Don't use! This code is broken
suspend inline fun <R> useTemporaryFile(block: suspend (File) -> R): R {
    var file: File? = null
    try {
        file = withContext(<http://Dispatchers.IO|Dispatchers.IO>) {
            createTempFile("foo", "temp")
        }
        return block(file)
    } finally {
        file?.delete()
    }
}

[trathschlag]

@uli if you just want to keep the original context, you could also use the magic coroutineContext extension property:

suspend fun <R> useTemporaryFile(body: suspend (File) -> R): R {
    val outerContext = coroutineContext
    return withContext(<http://Dispatchers.IO|Dispatchers.IO>) {
        val file = createTempFile("foo", "temp")

        try {
            withContext(outerContext) {
                body(file)
            }
        } finally {
            file.delete()
        }
    }
}

but I'm still not sure if there are any pitfalls regarding this.

and for the general problem: what about

suspend fun processImage() {
    val outerContext = coroutineContext

    withContext(NonCancellable) {
        val file = createFile()

        try {
            withContext(outerContext) {
                // do something with file
            }
        } finally {
            file.delete()
        }
    }
}

still not the most intuitive

[elizarov]

@uli This behavior of withContext is designed on purpose. In a UI app, when we return back to the main thread we want to be sure that the UI is not destroyed (job is not cancelled) before we attempt to manipulate it.

[elizarov]

We used to have an optional parameter to withContext control this behavior, but have dropped it, since it is too advanced and hard to use anyway.

[elizarov]

So, indeed, you cannot easily return a resource using withContext. The is very similar to the following problem:

fun createFile(): File =
  File.createTemporaryFile("foo", "temp")
  .also { doSomethingElse() } /// ooops... what if it fails? 
}

[ansman]

The app could also crash so I still have to clean up old files so not a huge issue, but could be a problem with other closable resources such as sockets and input streams. It could be great if there an option to have a block that would be called when the Coroutine would be resumed but is canceled

[elizarov]

We used to have it. We can have a separate version of withContext (maybe) with this kind of behavior.

[elizarov]

I am at loos at how to properly name it, though

[ansman]

The problem is not specific to withContext though but rather all functions that actually suspend

[elizarov]

The problem is not specific to functions that suspend, but to all functions that can throw exception.

[elizarov]

No all functions that suspend can throw an exception. The ones that can, are explicitly marked in their docs as being cancellable. Most of them do, though.

[ansman]

Fair enough, but it’s a problem with suspendCoroutine for example

[ansman]

At least with that one you can check if it was canceled

[elizarov]

this kind of pattern:

val r = acquireResource()
try { ... } finally { r.close }

is inherently tricky. You have to make there is nothing between acquireResource and try and the implementation of acquireResource need to have extreme care for exceptional cases.

[ansman]

Perhaps the solution is to have a try-finally pattern with Coroutines

[ansman]

When used it would guarantee that return values aren’t dropped

[elizarov]

Yes. We definitely need a better try/finally for coroutines. We have a similar problem with `launch`:

val r = acquireResource() 
launch { 
    try { ... } finally { r.close() }
}

It will not work, because launch may not even start.

[elizarov]

So we need some nice solution that works both for launch and for withContext

[ansman]

The launch example would when using ATOMIC though?

[elizarov]

Yes. Not a nice solution, but works. We used to have CoroutineStart paramater for withContext, too, but dropped it.

[elizarov]

We need a better solution for both cases