Skip to content

Commit 8e923f3

Browse files
[3.13] gh-91555: disable logger while handling log record (GH-131812) (GH-133898)
Co-authored-by: Duane Griffin <[email protected]>
1 parent 5cd56b2 commit 8e923f3

File tree

3 files changed

+105
-8
lines changed

3 files changed

+105
-8
lines changed

Lib/logging/__init__.py

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1474,6 +1474,8 @@ class Logger(Filterer):
14741474
level, and "input.csv", "input.xls" and "input.gnu" for the sub-levels.
14751475
There is no arbitrary limit to the depth of nesting.
14761476
"""
1477+
_tls = threading.local()
1478+
14771479
def __init__(self, name, level=NOTSET):
14781480
"""
14791481
Initialize the logger with a name and an optional level.
@@ -1670,14 +1672,19 @@ def handle(self, record):
16701672
This method is used for unpickled records received from a socket, as
16711673
well as those created locally. Logger-level filtering is applied.
16721674
"""
1673-
if self.disabled:
1674-
return
1675-
maybe_record = self.filter(record)
1676-
if not maybe_record:
1675+
if self._is_disabled():
16771676
return
1678-
if isinstance(maybe_record, LogRecord):
1679-
record = maybe_record
1680-
self.callHandlers(record)
1677+
1678+
self._tls.in_progress = True
1679+
try:
1680+
maybe_record = self.filter(record)
1681+
if not maybe_record:
1682+
return
1683+
if isinstance(maybe_record, LogRecord):
1684+
record = maybe_record
1685+
self.callHandlers(record)
1686+
finally:
1687+
self._tls.in_progress = False
16811688

16821689
def addHandler(self, hdlr):
16831690
"""
@@ -1765,7 +1772,7 @@ def isEnabledFor(self, level):
17651772
"""
17661773
Is this logger enabled for level 'level'?
17671774
"""
1768-
if self.disabled:
1775+
if self._is_disabled():
17691776
return False
17701777

17711778
try:
@@ -1815,6 +1822,11 @@ def _hierlevel(logger):
18151822
if isinstance(item, Logger) and item.parent is self and
18161823
_hierlevel(item) == 1 + _hierlevel(item.parent))
18171824

1825+
def _is_disabled(self):
1826+
# We need to use getattr as it will only be set the first time a log
1827+
# message is recorded on any given thread
1828+
return self.disabled or getattr(self._tls, 'in_progress', False)
1829+
18181830
def __repr__(self):
18191831
level = getLevelName(self.getEffectiveLevel())
18201832
return '<%s %s (%s)>' % (self.__class__.__name__, self.name, level)

Lib/test/test_logging.py

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4163,6 +4163,89 @@ def __init__(self, *args, **kwargs):
41634163
handler = logging.getHandlerByName('custom')
41644164
self.assertEqual(handler.custom_kwargs, custom_kwargs)
41654165

4166+
# See gh-91555 and gh-90321
4167+
@support.requires_subprocess()
4168+
def test_deadlock_in_queue(self):
4169+
queue = multiprocessing.Queue()
4170+
handler = logging.handlers.QueueHandler(queue)
4171+
logger = multiprocessing.get_logger()
4172+
level = logger.level
4173+
try:
4174+
logger.setLevel(logging.DEBUG)
4175+
logger.addHandler(handler)
4176+
logger.debug("deadlock")
4177+
finally:
4178+
logger.setLevel(level)
4179+
logger.removeHandler(handler)
4180+
4181+
def test_recursion_in_custom_handler(self):
4182+
class BadHandler(logging.Handler):
4183+
def __init__(self):
4184+
super().__init__()
4185+
def emit(self, record):
4186+
logger.debug("recurse")
4187+
logger = logging.getLogger("test_recursion_in_custom_handler")
4188+
logger.addHandler(BadHandler())
4189+
logger.setLevel(logging.DEBUG)
4190+
logger.debug("boom")
4191+
4192+
@threading_helper.requires_working_threading()
4193+
def test_thread_supression_noninterference(self):
4194+
lock = threading.Lock()
4195+
logger = logging.getLogger("test_thread_supression_noninterference")
4196+
4197+
# Block on the first call, allow others through
4198+
#
4199+
# NOTE: We need to bypass the base class's lock, otherwise that will
4200+
# block multiple calls to the same handler itself.
4201+
class BlockOnceHandler(TestHandler):
4202+
def __init__(self, barrier):
4203+
super().__init__(support.Matcher())
4204+
self.barrier = barrier
4205+
4206+
def createLock(self):
4207+
self.lock = None
4208+
4209+
def handle(self, record):
4210+
self.emit(record)
4211+
4212+
def emit(self, record):
4213+
if self.barrier:
4214+
barrier = self.barrier
4215+
self.barrier = None
4216+
barrier.wait()
4217+
with lock:
4218+
pass
4219+
super().emit(record)
4220+
logger.info("blow up if not supressed")
4221+
4222+
barrier = threading.Barrier(2)
4223+
handler = BlockOnceHandler(barrier)
4224+
logger.addHandler(handler)
4225+
logger.setLevel(logging.DEBUG)
4226+
4227+
t1 = threading.Thread(target=logger.debug, args=("1",))
4228+
with lock:
4229+
4230+
# Ensure first thread is blocked in the handler, hence supressing logging...
4231+
t1.start()
4232+
barrier.wait()
4233+
4234+
# ...but the second thread should still be able to log...
4235+
t2 = threading.Thread(target=logger.debug, args=("2",))
4236+
t2.start()
4237+
t2.join(timeout=3)
4238+
4239+
self.assertEqual(len(handler.buffer), 1)
4240+
self.assertTrue(handler.matches(levelno=logging.DEBUG, message='2'))
4241+
4242+
# The first thread should still be blocked here
4243+
self.assertTrue(t1.is_alive())
4244+
4245+
# Now the lock has been released the first thread should complete
4246+
t1.join()
4247+
self.assertEqual(len(handler.buffer), 2)
4248+
self.assertTrue(handler.matches(levelno=logging.DEBUG, message='1'))
41664249

41674250
class ManagerTest(BaseTest):
41684251
def test_manager_loggerclass(self):
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Ignore log messages generated during handling of log messages, to avoid
2+
deadlock or infinite recursion.

0 commit comments

Comments
 (0)