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

Introduce Dispatchers.shutdown #2915

Merged
merged 4 commits into from
Sep 9, 2021
Merged

Introduce Dispatchers.shutdown #2915

merged 4 commits into from
Sep 9, 2021

Conversation

qwwdfsad
Copy link
Contributor

@qwwdfsad qwwdfsad commented Sep 3, 2021

Fixes #2558

Copy link
Collaborator

@dkhalanskyjb dkhalanskyjb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comments I made in #2903 (review) are somewhat applicable here: I think that we need to establish and document some specific behavior for cases where there are some pending jobs at the moment of shutting down the dispatchers.

As is, I managed to get a highly confusing and uninformative error with this code:

        val j = GlobalScope.launch {
            delay(1000)
            println("won't be reached")
        }
        GlobalScope.launch {
            Dispatchers.shutdown()
            j.join()
        }

The error:

Exception in thread Thread[DefaultDispatcher-worker-1 @coroutine#1,5,main]: java.lang.AssertionError
java.lang.AssertionError
	at kotlinx.coroutines.EventLoopImplPlatform.reschedule(EventLoop.kt:17)
	at kotlinx.coroutines.EventLoopImplBase.schedule(EventLoop.common.kt:362)
	at kotlinx.coroutines.EventLoopImplBase.scheduleResumeAfterDelay(EventLoop.common.kt:235)
	at kotlinx.coroutines.DelayKt.delay(Delay.kt:121)
	at kotlinx.coroutines.RunBlockingTest$dispatcherShutdownTest$j$1.invokeSuspend(RunBlockingTest.kt:15)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106)
	at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:571)
	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:750)
	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:678)
	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:665)

@qwwdfsad
Copy link
Contributor Author

qwwdfsad commented Sep 6, 2021

I've improved the exception message and documentation.

The API is marked as delicate and has a pretty narrow use case, so I'm not sure it's worth (both in terms of contract and maintainability) giving more guarantees than now.

Co-authored-by: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com>
# Conflicts:
#	kotlinx-coroutines-core/jvm/src/CoroutineContext.kt
#	kotlinx-coroutines-core/jvm/test/TestBase.kt
@qwwdfsad qwwdfsad merged commit 1786eb0 into develop Sep 9, 2021
@qwwdfsad qwwdfsad deleted the dispatchers-shutdown branch September 9, 2021 16:11
pablobaxter pushed a commit to pablobaxter/kotlinx.coroutines that referenced this pull request Sep 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants