gh-150818: Wire logger parent before publishing it in getLogger() (GH-150941)#150941
Merged
vsajip merged 2 commits intoJun 5, 2026
Merged
Conversation
getLogger() returns existing loggers through a lock-free fast path that reads loggerDict without the lock (added in pythonGH-150825). The slow path inserted a freshly created logger into loggerDict before _fixupParents() assigned its parent, so a concurrent caller could fetch a logger whose parent was still None and see it detached from the hierarchy: getEffectiveLevel() returns NOTSET and records do not propagate until the creating thread finishes. Publish the logger only after its parent and children are wired. Neither _fixupParents() nor _fixupChildren() reads loggerDict[name], so the reorder is safe; the fast path is unchanged and confined to the locked creation path taken once per logger.
Member
|
The new test is failing under Emscripten/WASI due to an inability to start a new thread - I'm not sure if this a transient problem or a platform restriction of those environments. It may be an idea to skip that test in those environments, at least for now. |
WASI and Emscripten have no working threading, so guard the regression test with requires_working_threading(), matching the other thread-using tests in test_logging.
vsajip
approved these changes
Jun 5, 2026
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.
The lock-free fast path added in GH-150825 reads
loggerDictwithout the lock and returns any non-placeholder logger it finds. The slow path, however, inserted a freshly created logger intologgerDictbefore calling_fixupParents(), which assigns itsparent. A caller hitting the fast path during that window receives a logger whoseparentis stillNone:getEffectiveLevel()returnsNOTSETand records do not propagate to ancestors until the creating thread finishes.Both slow-path branches now publish the logger to
loggerDictonly after_fixupParents()(and_fixupChildren()for the placeholder case) have wired it. Neither helper readsloggerDict[name], so the move is safe, and the fast path is unchanged.The added regression test forces the publish/wire interleaving deterministically and fails without this change.
The reorder is confined to the locked creation path, so the fast path is unaffected. Median of 5 runs against GH-150825 as merged (PGO build, same interpreter, swapping
Lib/logging/__init__.py):The difference is within the run-to-run noise floor (base 2.66-2.76 us, patched 2.64-2.65 us);
pyperf compare_toreports it as not significant.Benchmark (pyperf)
Base is
Lib/logging/__init__.pyat GH-150825; patched swaps in this PR's version on the same interpreter. Run base vs patched withpyperf compare_to base.json patched.json.Thanks to @methane for catching this in review of GH-150825.