Kotlin/kotlinx.coroutines

Coroutine context lost easily between calls

whirvis opened this issue · 4 comments

Describe the bug

When passing a Job as a CoroutineContext, the rest of the context is lost. I have a code fragment which produces unexpected output. When running it, I expect the following output:

CoroutineName(foo)
    CoroutineName(bar)
    CoroutineName(baz)
    CoroutineName(qux)
579

But instead, I get this:

CoroutineName(foo)
    DeferredCoroutine{Active}@2bea5ab4
    DeferredCoroutine{Active}@3d8314f0
    DeferredCoroutine{Active}@2df32bf7
579

After some digging, I realized this is because CoroutineContext.Element (implemented by Job) also implements CoroutineContext. In the implementation for CoroutineContext.Element, we find the following:

public override operator fun <E : Element> get(key: Key<E>): E? =
       @Suppress("UNCHECKED_CAST")
       if (this.key == key) this as E else null

Personally, I find this behavior very confusing and unexpected. Is there a reason why CoroutineContext.Element implements the CoroutineContext interface? (also: if the code fragment I gave is not optimal, please state so. I am new to Kotlin coroutines and I want to use them correctly.)

What is the desired behavior? Do you want the it.print(indent + 4) line to cause a compilation failure instead because it's not a CoroutineContext of any scope but instead just a single context element?

If so, the recommended approach is to have CoroutineScope as the receiver of your print function. Semantically, the CoroutineContext in a CoroutineScope is expected to contain the full coroutine context and not just one element.

fun CoroutineScope.print(indent: Int = 0) {
    print(" ".repeat(indent))
    println(coroutineContext[CoroutineName] ?: coroutineContext.job)
    coroutineContext.job?.children?.forEach { 
        (it as CoroutineScope)?.print(indent + 4)
    }
}

The overall recommended approach is to consider using kotlinx-coroutines-debug to print the coroutine hierarchy (see https://github.com/Kotlin/kotlinx.coroutines/tree/master/kotlinx-coroutines-debug#example-of-usage) instead of manually traversing the coroutine context.

Is there a reason why CoroutineContext.Element implements the CoroutineContext interface?

Yes. This way, we can write both async(CoroutineName("bar")) { } (adding a single coroutine context element) and async(CoroutineName("bar") + Dispatchers.Default) { } (adding a compound coroutine context) using a single overload that accepts CoroutineContext.

Hey! Thank you for replying, I'm sorry I took so long to respond to this one. I think the first option (cause a compilation failure) or return the coroutines bar, baz, and qux. was what I was expecting. I did manage to get the expected output by changing my code a little, and it eventually resulted in this:

fun CoroutineContext.print(
    stream: PrintStream,
    indent: Int = 4,
    level: Int = 0,
) {
    val job = this[Job]
    val name = this[CoroutineName]

    stream.print(" ".repeat(indent * level))
    stream.println(name ?: job)

    job?.children?.forEach {
        val scope = (it as? CoroutineScope)
        scope?.coroutineContext?.print(stream, indent, level + 1)
    }
}

Is this an okay way to do it, or should I be doing it some other way? (I realize it's very similar to the code sample you posted, but I still wanted to make sure it was okay).

I know you mentioned kotlinx-coroutines-debug to print the hierarchy, but I intend to use this in release applications. Specifically, to show the user what things the program is currently doing (e.g., loading a large file, waiting for a network response, etc.) Also, from what I can tell, I should use CopyableThreadContextElement to create custom context elements for extra data. Am I correct? I want to add data to the context so I can show something like "Loading data..." instead of CoroutineContext(foo).

Your code is okay, yes.

For your use case, I'd recommend not using kotlinx.coroutines machinery at all, instead implementing your own tracker of subtasks, like

scope.launch {
    subtaskTracker.enter("Loading data") {
        // load data here
    }
}

Your users shouldn't typically need to know about your concurrent decomposition of tasks, it's an implementation detail. Coroutines are much more fine-grained than operations like "waiting for a network response": you could have, say, four coroutines, one of which orchestrates the process of accessing the network, another watches the elapsed time to perform a timeout, the third tries to access the cache in parallel to see if the page was recently queried already, with the final coroutine actually performing the network call. If you tie your output to the coroutines machinery, you won't be able to decompose your work into several coroutines without overloading the user with unnecessary information.

Also, please consider using Stack Overflow or the Kotlinlang Slack. There, you can expect to get an answer much sooner. This is a place where we track issues with kotlinx.coroutines, and usage questions may get lost or forgotten about, which is what happened this time as well, sorry about that.

I see, thank you very much for your detailed response!