Fixing Indeterminate Behavior Caused by InvocationKind.EXACTLY_ONCE

[Daiji256]

The runCatching function was initially set to InvocationKind.EXACTLY_ONCE, which could lead to indeterminate behavior. Therefore, it has been changed to InvocationKind.AT_MOST_ONCE.

The problem occurs in the following case:

@Test
fun indeterminate() {
    val value: Int
    com.github.michaelbull.result.runCatching {
        throwableFunction()
        value = 1
    }

    assertEquals(
        expected = 0,
        actual = value,
    )
}

With InvocationKind.EXACTLY_ONCE, it is assumed that value = 1 is executed. However, in practice, this may not happen, resulting in an indeterminate value (0).

[michaelbull]

Thanks for the PR. Please can you explain why EXACTLY_ONCE is wrong? The InvocationKind contract specifically refers to the amount of times that the function parameter is called, not every line in that function.

EXACTLY_ONCE
"A function parameter will be invoked exactly one time."

From my understanding, your description that "it is assumed that value = 1 is executed" is not correct. The InvocationKind contract is not suggesting that every line of code is invoked once, but rather that the function passed to runCatching is called once: which it is. There is no situation in which runCatching will invoke the function parameter more than once, and no situation in which it won't be executed.

If it works as you described, then I don't see how anyone could ever write a contract that truly executes a function parameter EXACTLY_ONCE (with all lines in the function being executed) - as any function parameter can be passed code that throws an exception.

---

Consider rewriting your example in the following format:

private var value: Int = 0

fun throwableFunction() = throw RuntimeException()

fun throwSomething() {
    throwableFunction()
    value = 1
}

@Test
fun indeterminate() {
    val value: Int
    com.github.michaelbull.result.runCatching(::throwSomething)

    assertEquals(
        expected = 0,
        actual = value,
    )
}

In the example above, throwSomething is explicitly called EXACTLY_ONCE by runCatching. The contract has nothing to do with what happens inside the throwSomething function itself, but that runCatching invoked it exactly one time.

[Daiji256]

There is no contract set in kotlin.result. Therefore, the following implementation cannot even be compiled and will also be warned by the IDE.

My understanding is that contract is used to inform the compiler that the code works without issues. To avoid implementing uncertain behavior by mistake, I believe it is better not to specify EXACTLY_ONCE.

@Test
fun kotlin_runCatching() {
    val value: Int
    kotlin.runCatching {
        throwableFunction()
        value = 1 // Captured values initialization is forbidden due to possible reassignment
    }

    assertEquals(
        expected = 1,
        actual = value, // Variable 'value' must be initialized
    )
}

[michaelbull]

Does that example picture compile without the contract? If we don't tell the compiler it will be executed at all with an InvocationKind contract, then how does it know that value = 1 is ever called?

[Daiji256]

The example with kotlin.runCatching cannot be compiled without the contract. The compiler does not know whether value = 1 is executed.

In runCatching, the subsequent process will be executed regardless of the result of the block. In other words, the subsequent process has similar characteristics to the finally block in a try-finally statement.

@Test
fun try_finaly() {
    val value: Int

    try {
        throwableFunction()
        value = 1
    } finally {
        // No guarantee that `value = 1` was executed
        assertEquals(
            expected = 1,
            actual = value, // Variable 'value' must be initialized
        )
    }

    // Guaranteed that `value = 1` was executed
    assertEquals(
        expected = 1,
        actual = value, // No Error
    )
}

When using contract + EXACTLY_ONCE, the compiler will assume that the block has been successfully processed, and the subsequent process will be executed accordingly.

To prevent the compiler from mistakenly assuming that the block has completed successfully, I believe that com.github.michaelbull.result.runCatching should avoid using contract + EXACTLY_ONCE.

[nikclayton]

Looking at this:

@Test
fun kotlin_runCatching() {
    val value: Int
    kotlin.runCatching {
        throwableFunction()
        value = 1 // Captured values initialization is forbidden due to possible reassignment
    }

    assertEquals(
        expected = 1,
        actual = value, // Variable 'value' must be initialized
    )
}

@Daiji256 -- is it possible you've misunderstood what runCatching does? It doesn't ignore the exception in the block and carry on with the next operation, it returns the value of the block as a Result.

So this example would be better written as something like:

@Test
fun kotlin_runCatching() {
    // value is Result<Int, Throwable>
    val value = kotlin.runCatching {
        throwableFunction()
        1
    }

    assertEquals(
        expected = 1,
        actual = value.get() // returns 1, or null if the result is an error because throwableFunction() threw
    )
}

[Daiji256]

I'm sorry if my English is unclear.

What I believe to be the issue is "the possibility of indeterminate behavior." I don't intend to discuss how to implement the example code without bugs.

If the following code is removed, it will be beneficial because it will prevent the possibility of implementing it in a way that leads to indeterminate behavior. The proposed code can still be implemented without this code.

contract {
    callsInPlace(block, InvocationKind.EXACTLY_ONCE)
}