Skip to content

Commit 6b62522

Browse files
committed
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.
1 parent e833f57 commit 6b62522

2 files changed

Lines changed: 46 additions & 5 deletions

File tree

Lib/logging/__init__.py

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1377,9 +1377,10 @@ def getLogger(self, name):
13771377
raise TypeError('A logger name must be a string')
13781378
# Fast path: an already-registered, non-placeholder logger can be
13791379
# returned without taking the lock. dict.get() is atomic under both
1380-
# the GIL and free threading, and a Logger is fully initialised before
1381-
# being inserted into loggerDict under the lock, so this never sees a
1382-
# partially-constructed object.
1380+
# the GIL and free threading. A Logger is inserted into loggerDict only
1381+
# after it is fully wired up (parent/child references fixed) under the
1382+
# lock, so the fast path never observes a logger whose parent is not yet
1383+
# set.
13831384
rv = self.loggerDict.get(name)
13841385
if rv is not None and not isinstance(rv, PlaceHolder):
13851386
return rv
@@ -1390,14 +1391,18 @@ def getLogger(self, name):
13901391
ph = rv
13911392
rv = (self.loggerClass or _loggerClass)(name)
13921393
rv.manager = self
1393-
self.loggerDict[name] = rv
13941394
self._fixupChildren(ph, rv)
13951395
self._fixupParents(rv)
1396+
# Publish only after rv is fully wired: the fast path reads
1397+
# loggerDict without the lock.
1398+
self.loggerDict[name] = rv
13961399
else:
13971400
rv = (self.loggerClass or _loggerClass)(name)
13981401
rv.manager = self
1399-
self.loggerDict[name] = rv
14001402
self._fixupParents(rv)
1403+
# Publish only after rv is fully wired: the fast path reads
1404+
# loggerDict without the lock.
1405+
self.loggerDict[name] = rv
14011406
return rv
14021407

14031408
def setLoggerClass(self, klass):

Lib/test/test_logging.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4269,6 +4269,42 @@ def test_set_log_record_factory(self):
42694269
man.setLogRecordFactory(expected)
42704270
self.assertEqual(man.logRecordFactory, expected)
42714271

4272+
def test_getLogger_fast_path_never_returns_unwired_logger(self):
4273+
# getLogger()'s lock-free fast path returns a logger straight out of
4274+
# loggerDict, so a logger must be published there only after
4275+
# _fixupParents() has set its parent; otherwise a concurrent caller
4276+
# observes it detached from the hierarchy (gh-150818 follow-up).
4277+
manager = logging.Manager(logging.RootLogger(logging.WARNING))
4278+
name = 'a.b.c'
4279+
4280+
paused = threading.Event()
4281+
seen = []
4282+
real_fixup = manager._fixupParents
4283+
4284+
# Pause the creating thread between publishing rv and wiring its
4285+
# parent, then read loggerDict the way the fast path does and snapshot
4286+
# the parent at that instant (rv is wired in place soon after).
4287+
def fixup(alogger):
4288+
paused.set()
4289+
reader.join()
4290+
real_fixup(alogger)
4291+
4292+
def read():
4293+
paused.wait()
4294+
rv = manager.loggerDict.get(name)
4295+
if rv is not None and not isinstance(rv, logging.PlaceHolder):
4296+
seen.append(rv.parent)
4297+
4298+
reader = threading.Thread(target=read)
4299+
manager._fixupParents = fixup
4300+
try:
4301+
reader.start()
4302+
manager.getLogger(name)
4303+
finally:
4304+
manager._fixupParents = real_fixup
4305+
4306+
self.assertNotIn(None, seen)
4307+
42724308
class ChildLoggerTest(BaseTest):
42734309
def test_child_loggers(self):
42744310
r = logging.getLogger()

0 commit comments

Comments
 (0)