From 6b625229ed1ad0a3f09f88ca2e00d0c75cd17577 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bern=C3=A1t=20G=C3=A1bor?= Date: Thu, 4 Jun 2026 22:07:05 -0700 Subject: [PATCH 1/2] gh-150818: Wire logger parent before publishing it getLogger() returns existing loggers through a lock-free fast path that reads loggerDict without the lock (added in GH-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. --- Lib/logging/__init__.py | 15 ++++++++++----- Lib/test/test_logging.py | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/Lib/logging/__init__.py b/Lib/logging/__init__.py index 9febc50b1264ef..b4a5f5cc2f598f 100644 --- a/Lib/logging/__init__.py +++ b/Lib/logging/__init__.py @@ -1377,9 +1377,10 @@ def getLogger(self, name): raise TypeError('A logger name must be a string') # Fast path: an already-registered, non-placeholder logger can be # returned without taking the lock. dict.get() is atomic under both - # the GIL and free threading, and a Logger is fully initialised before - # being inserted into loggerDict under the lock, so this never sees a - # partially-constructed object. + # the GIL and free threading. A Logger is inserted into loggerDict only + # after it is fully wired up (parent/child references fixed) under the + # lock, so the fast path never observes a logger whose parent is not yet + # set. rv = self.loggerDict.get(name) if rv is not None and not isinstance(rv, PlaceHolder): return rv @@ -1390,14 +1391,18 @@ def getLogger(self, name): ph = rv rv = (self.loggerClass or _loggerClass)(name) rv.manager = self - self.loggerDict[name] = rv self._fixupChildren(ph, rv) self._fixupParents(rv) + # Publish only after rv is fully wired: the fast path reads + # loggerDict without the lock. + self.loggerDict[name] = rv else: rv = (self.loggerClass or _loggerClass)(name) rv.manager = self - self.loggerDict[name] = rv self._fixupParents(rv) + # Publish only after rv is fully wired: the fast path reads + # loggerDict without the lock. + self.loggerDict[name] = rv return rv def setLoggerClass(self, klass): diff --git a/Lib/test/test_logging.py b/Lib/test/test_logging.py index 31c052bfb56cd7..df3cea900e121a 100644 --- a/Lib/test/test_logging.py +++ b/Lib/test/test_logging.py @@ -4269,6 +4269,42 @@ def test_set_log_record_factory(self): man.setLogRecordFactory(expected) self.assertEqual(man.logRecordFactory, expected) + def test_getLogger_fast_path_never_returns_unwired_logger(self): + # getLogger()'s lock-free fast path returns a logger straight out of + # loggerDict, so a logger must be published there only after + # _fixupParents() has set its parent; otherwise a concurrent caller + # observes it detached from the hierarchy (gh-150818 follow-up). + manager = logging.Manager(logging.RootLogger(logging.WARNING)) + name = 'a.b.c' + + paused = threading.Event() + seen = [] + real_fixup = manager._fixupParents + + # Pause the creating thread between publishing rv and wiring its + # parent, then read loggerDict the way the fast path does and snapshot + # the parent at that instant (rv is wired in place soon after). + def fixup(alogger): + paused.set() + reader.join() + real_fixup(alogger) + + def read(): + paused.wait() + rv = manager.loggerDict.get(name) + if rv is not None and not isinstance(rv, logging.PlaceHolder): + seen.append(rv.parent) + + reader = threading.Thread(target=read) + manager._fixupParents = fixup + try: + reader.start() + manager.getLogger(name) + finally: + manager._fixupParents = real_fixup + + self.assertNotIn(None, seen) + class ChildLoggerTest(BaseTest): def test_child_loggers(self): r = logging.getLogger() From 01626e1cfe680d3b99b371ac204afd26ec749757 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bern=C3=A1t=20G=C3=A1bor?= Date: Fri, 5 Jun 2026 06:58:45 -0700 Subject: [PATCH 2/2] gh-150818: Skip new test where threading is unavailable 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. --- Lib/test/test_logging.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/test_logging.py b/Lib/test/test_logging.py index df3cea900e121a..a6f5f05bd9834f 100644 --- a/Lib/test/test_logging.py +++ b/Lib/test/test_logging.py @@ -4269,6 +4269,7 @@ def test_set_log_record_factory(self): man.setLogRecordFactory(expected) self.assertEqual(man.logRecordFactory, expected) + @threading_helper.requires_working_threading() def test_getLogger_fast_path_never_returns_unwired_logger(self): # getLogger()'s lock-free fast path returns a logger straight out of # loggerDict, so a logger must be published there only after