Which version is correct? ```suspend fun foo() { ...

# coroutines

[Richard Schielek]

Which version is correct?

suspend fun foo() {
    MDC.put("key", "value")
    withContext(MDCContext()) {
        bar()
    }
}

suspend fun bar() = withContext(Dispatchers.Default) {
    // ...
}

vs

suspend fun foo() {
    MDC.put("key", "value")
    bar()
}

suspend fun bar() = withContext(Dispatchers.Default + MDCContext()) {
    // ...
}

[Mario Niebes]

As described here
https://kotlinlang.org/api/kotlinx.coroutines/kotlinx-coroutines-slf4j/kotlinx.coroutines.slf4j/-m-d-c-context/

[Richard Schielek]

I read this prior to asking my question. The examples don't cover best practices when code is split across functions.
I would say the function modifying the MDC is also responsible for ensuring it's propagated correctly.

[kevin.cianfarini]

Typically you want the coroutine context a function executes on to be an implementation detail of that function. In your example, it seems like bar should execute on both the default dispatcher and the MDCContext. For that reason the second sample is not a leaky implementation and should be preferred.

[kevin.cianfarini]

This is an extension of the concept that functions should be main safe from the mobile development world.

Suspend functions should be main-safe, meaning they’re safe to call from the main thread. If a class is doing long-running blocking operations in a coroutine, it’s in charge of moving the execution off the main thread using withContext. This applies to all classes in your app, regardless of the part of the architecture the class is in.

[Richard Schielek]

I am aware of this rule. That's why in both examples the dispatcher is changed in bar. But it doesn't answer which function needs to ensure the MDC is propagated.

[Daniel Pitts]

If bar doesn't directly care about the MDC, then it shouldn't care about the MDCContext.

[Richard Schielek]

After thinking this through again I came to the following conclusion:
A suspension could theoretically switch threads. bar cannot rely on being called right after the MDC was modified. foo on the other hand has full knowledge of the MDC being modified. It needs to ensure the MDC is propagated correctly.

Conclusion: First variant where foo is setting the MDCContext the is correct. Second variant where bar is setting it is error-prone.

Example

// correct
suspend fun foo() {
    MDC.put("key", "value")
    // must be called before the next suspension point
    withContext(MDCContext()) {
        delay(500)
        bar()
    }
}

suspend fun bar() = withContext(Dispatchers.Default) {
    // ...
}

vs

suspend fun foo() {
    MDC.put("key", "value")
    delay(500) // this will break
    bar()
}

suspend fun bar() = withContext(Dispatchers.Default + MDCContext()) {
    // ...
}

[Hugo Costa]

I've gone with this pattern (this is one of my APIs)

fun validateNetwork(input: ValidateNetworkInput) = runBlocking(Dispatchers.Default + MDCContext()) {
        MDC.put("externalValidationID", input.validationUniqueIdentifier)

        withContext(MDCContext()) {
            validateNetworkHandler.handle(input, identity)
        }
    }

And within the handle function

suspend fun handle(input: ValidateNetworkInput, identity: Identity): ValidateNetworkOutput = coroutineScope {
        stuff
    }

It's important to do the coroutineScope otherwise the map gets lost btw

[Richard Schielek]

It's important to do the coroutineScope otherwise the map gets lost btw

Are you sure about that? To my understanding only withContext(MDCContext()) is relevant. You should be able to omit it in runBlocking and you shouldn't be forced to create a new scope for propagating the MDC.

[Hugo Costa]

My bad - was confusing, shouldn't be required (unless you're planning on creating child launch/async