Try-finally and non-local returns - Mutex.withLock

[Laxystem]

Here is the current implementation of KotlinX Coroutines Mutex.withLock():

/**
 * Executes the given [action] under this mutex's lock.
 *
 * @param owner Optional owner token for debugging. When `owner` is specified (non-null value) and this mutex
 *        is already locked with the same token (same identity), this function throws [IllegalStateException].
 *
 * @return the return value of the action.
 */
@OptIn(ExperimentalContracts::class)
public suspend inline fun <T> Mutex.withLock(owner: Any? = null, action: () -> T): T {
    contract {
        callsInPlace(action, InvocationKind.EXACTLY_ONCE)
    }
    lock(owner)
    return try {
        action()
    } finally {
        unlock(owner)
    }
}

Notice how action is not crossinline. This means that:

suspend fun foo() = mutex.withLock() {
    return 42 // bad, mutex doesn't unlock
}

However, action is called inside a try block - do finallys apply to non-local returns?
If so, maybe there should be something in the language, or at the very least, in the function's documentation that tells me about it?

This is very confusing, and potentially could cause people to switch returns with return@withLock all over their project, and then realize that the try block automagically applies to non-local-returns. This feels very goto-y.

This issue was previously submitted as KT-66567.

[mgroth0]

This issue has me a bit worried but also a bit confused. You ask "do finallys apply to non-local returns?", but wouldn't this take a minute to test? My understanding was that code inside a finally block always executes, no matter what.

If you could create a small reproducer of a case where code inside of a finally is not executed, I would like to see it.

[mgroth0]

fun main() {
    val mutex = Mutex()
    suspend fun foo(): Int =
        mutex.withLock {
            return 42
        }
    runBlocking {
        val result = foo()
        println("result=$result")
        println("isLocked=${mutex.isLocked}")
    }
}

Output:

result=42
isLocked=false

I am confused if you have ever observed a finally block not running, or if this is just a misunderstanding.

[Laxystem]

I'm saying the documentation should be changed, because this can be confusing.

[dkhalanskyjb]

This is not a problem with the library: you're not even supposed to read the source code to use it, so "I don't understand why the implementation works properly" is not something we consider a bug. Especially, we are not going to write comments about fundamental language constructs like try and return. Here's a good explanation of how finally works: https://docs.oracle.com/javase/tutorial/essential/exceptions/finally.html and here's an explanation of inline functions: https://kotlinlang.org/docs/inline-functions.html