Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CoroutineDispatcher.dispatch ignores the context parameter. #3354

Closed
bc-lee opened this issue Jun 30, 2022 · 2 comments
Closed

CoroutineDispatcher.dispatch ignores the context parameter. #3354

bc-lee opened this issue Jun 30, 2022 · 2 comments
Assignees
Labels
docs KDoc and API reference

Comments

@bc-lee
Copy link

bc-lee commented Jun 30, 2022

According to the link and source, CoroutineDispatcher.dispatch has the following behavior:

Dispatches execution of a runnable [block] onto another thread in the given [context]

But from what I've tested, CoroutineDispatcher.dispatch ignores the context parameter.

For example,

Click to toggle contents of `code`
@OptIn(ExperimentalCoroutinesApi::class)
class SerialTaskQueue {
    private class CoroutineQueueDepthContext(val taskQueue: SerialTaskQueue) :
        ThreadContextElement<Unit> {
        companion object Key : CoroutineContext.Key<CoroutineQueueDepthContext>

        override val key: CoroutineContext.Key<CoroutineQueueDepthContext>
            get() = Key

        // this is invoked before coroutine is resumed on current thread
        override fun updateThreadContext(context: CoroutineContext) {
            taskQueue.queueDepth.set(1)
        }

        // this is invoked after coroutine has suspended on current thread
        override fun restoreThreadContext(context: CoroutineContext, oldState: Unit) {
            taskQueue.queueDepth.set(0)
        }
    }

    private val dispatcher = Dispatchers.IO.limitedParallelism(1)

    private var queueDepth = ThreadLocal<Int>()

    fun post(action: () -> Unit) {
        dispatcher.dispatch(CoroutineQueueDepthContext(this), action)
    }

    fun post2(action: () -> Unit) {
        CoroutineScope(EmptyCoroutineContext).launch(
                dispatcher + CoroutineQueueDepthContext(this)) { action() }
    }

    fun isCurrent(): Boolean {
        return (queueDepth.get() ?: 0) > 0
    }
}

fun main() {
    val taskQueue = SerialTaskQueue()
    val latch = CountDownLatch(2)
    taskQueue.post {
        println("taskQueue.post isCurrent: ${taskQueue.isCurrent()}")
        latch.countDown()
    }
    taskQueue.post2 {
        println("taskQueue.post2 isCurrent: ${taskQueue.isCurrent()}")
        latch.countDown()
    }
    latch.await()
}

The code above will return something like this:

taskQueue.post isCurrent: false
taskQueue.post2 isCurrent: true

However, according to the documentation, both method calls should output true.

The above tests used org.jetbrains.kotlin:kotlin-stdlib-common:1.7.0 and org.jetbrains.kotlinx:kotlinx-coroutines-core:1.6.3 in JVM.

@qwwdfsad qwwdfsad self-assigned this Jun 30, 2022
@qwwdfsad qwwdfsad added the docs KDoc and API reference label Jun 30, 2022
@qwwdfsad
Copy link
Collaborator

Thanks for spotting!

This is indeed an expected behaviour: coroutine context elements affect coroutines and dispatcher.dispatch dispatches not coroutines, but Runnables. Moreover, due to the nature of how dispatchers (or, in general, executors) work, dispatchers cannot set up a context, it can only properly pass the given Runnable through the system so it will eventually execute and take care of its own context.

We definitely need better documentation here, context parameter in dispatch method indicates a context of a coroutine being dispatched (if any), so it can make additional decisions such as dispatching priority

@bc-lee
Copy link
Author

bc-lee commented Jun 30, 2022

Thanks for the quick response and thanks for clarifying this issue. I also agree that the documentation could be better.

Looking at the code, it seems that the context parameter is ignored in many implementations. So, in my opinion, it's worth mentioning in the documentation that the context parameter may be ignored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs KDoc and API reference
Projects
None yet
Development

No branches or pull requests

2 participants