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

Consolidate coroutines machinery for "regular" code paths and code paths for continuations that do not use kotlinx.coroutines-based ContinuationInterceptor #3253

Open
qwwdfsad opened this issue Apr 13, 2022 · 0 comments

Comments

@qwwdfsad
Copy link
Member

Currently, the machinery of coroutines have two code-paths:

  • Dispatcheable coroutines that have ContinuationInteceptor being an instance of CoroutineDispatcher in their context. From implementation standpoint, implementation checks (one more example for the reference) whether continuation is an instance of DispatchedContinuation and proceeds to a dispatcher.
  • Everything else, that is handled as plain old continuation.resumeWith.

Such approach tend to work perfectly fine, but over the course of time the invariant and the purpose of DispatchedContinuation have been changed: not only it is a cacheable wrapper for Runnable, but it also stores thread context values, is responsible for updating thread locals, forming an event loop when necessary and controlling the cancellation.

While implementing these changes, the second code path was never properly adapted to these changes and as they never seem to be related to this code path. #2930 has proven this reasoning wrong and the fix (#3252) does not really prevents similar future bugs, but with slightly different coroutine builders.

Potential solution is to wrap any continuation into DispatchedContinuation, unify all supplementary machinery between these two code-paths and attempt to unify run and resumeWith invariants (it requires an additional careful research for that).
The change will be intrusive and it's likely to touch the very core of the coroutines, so it may be worth postponing until we'll find more than one problem with the current approach

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant