fix(context): root_context() is the main Scope's context#189
Merged
Conversation
root_context() kept its context in a global of its own, ASYNC_G(root_context), created
lazily by the function itself and never attached to any Scope. find() resolves a key by
walking context->scope->parent_scope, so a context that belongs to no Scope was
unreachable by construction: root_context()->set() was only ever readable by calling
root_context() again.
It is the main Scope's context, which is exactly what the docblock ("Returns the root
Scope") already said. Now that it is, everything falls out of the Scope tree:
root_context() === current_context() at top level, and every Scope chain terminates at
the main Scope, so find() reaches it from any coroutine that inherits from it.
A plain `new Scope()` is a detached root on purpose (async_new_scope(NULL)), so its
coroutines still do not inherit the context -- Scope::inherit() is the API for that, and
it does reach the root. Those coroutines can still read it via root_context() directly.
The global is gone rather than kept in sync, so there is no second place for a root
context to live.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
CHANGELOG: both entries kept. async_arginfo.h: regenerated from the merged stub rather than resolved by hand, so it stays in step with its source.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Item 2 of the tutorial bug reports. The docs were right; the code was not.
Symptom
root_context()is invisible tofind()from anywhere:And
root_context() !== current_context()at top level, which it should be — they are two different objects.Cause
root_context()kept its context in a global of its own —ASYNC_G(root_context)— created lazily by the function itself and never attached to any Scope.find()resolves a key by walkingcontext->scope->parent_scopeand checking each Scope's context. A context that belongs to no Scope is therefore unreachable by construction: whatever you write to it is readable only by callingroot_context()again.So there were two parallel roots — the main Scope's context (which coroutines do see) and this global (which nothing sees).
Fix
The root context is the main Scope's context — which is what the docblock already said ("Returns the root Scope"). Making it so, everything falls out of the Scope tree with no extra machinery:
root_context() === current_context()at top level;scope.cgives a parentless ScopeMAIN_SCOPEas parent), sofind()reaches it from any coroutine that inherits from it.The
ASYNC_G(root_context)global is removed rather than kept in sync, so there is no second place for a root context to live.Semantics, verified
root_context() === current_context()at top leveltruefind('cfg')'X'Scope::inherit(),find('cfg')'X'new Scope(),find('cfg')NULLnew Scope(),root_context()->find('cfg')'X'A plain
new Scope()is a detached root on purpose (async_new_scope(NULL, …)) — that is the whole reasonScope::inherit()exists as a separate API — so its coroutines do not inherit the context either. They still reach the root by callingroot_context()directly. The stub docblocks are updated to say this precisely instead of "visible from any context", which was the overstatement in the docs.Tests
New
tests/context/010-root_context_is_main_scope.phptpins all five rows above.Full
ext/async/tests: no new failures. The 3 that fail (curl/063,curl/064,io/082) fail identically on a pristine baseline.