Skip to content

Commit 2561e14

Browse files
authored
gh-91555: disable logger while handling log record (GH-131812)
Prevent the possibility of re-entrancy leading to deadlock or infinite recursion (caused by logging triggered by logging), by disabling logging while the logger is handling log messages.
1 parent 8bb9286 commit 2561e14

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
@@ -4214,6 +4214,89 @@ def __init__(self, *args, **kwargs):
42144214
handler = logging.getHandlerByName('custom')
42154215
self.assertEqual(handler.custom_kwargs, custom_kwargs)
42164216

4217+
# See gh-91555 and gh-90321
4218+
@support.requires_subprocess()
4219+
def test_deadlock_in_queue(self):
4220+
queue = multiprocessing.Queue()
4221+
handler = logging.handlers.QueueHandler(queue)
4222+
logger = multiprocessing.get_logger()
4223+
level = logger.level
4224+
try:
4225+
logger.setLevel(logging.DEBUG)
4226+
logger.addHandler(handler)
4227+
logger.debug("deadlock")
4228+
finally:
4229+
logger.setLevel(level)
4230+
logger.removeHandler(handler)
4231+
4232+
def test_recursion_in_custom_handler(self):
4233+
class BadHandler(logging.Handler):
4234+
def __init__(self):
4235+
super().__init__()
4236+
def emit(self, record):
4237+
logger.debug("recurse")
4238+
logger = logging.getLogger("test_recursion_in_custom_handler")
4239+
logger.addHandler(BadHandler())
4240+
logger.setLevel(logging.DEBUG)
4241+
logger.debug("boom")
4242+
4243+
@threading_helper.requires_working_threading()
4244+
def test_thread_supression_noninterference(self):
4245+
lock = threading.Lock()
4246+
logger = logging.getLogger("test_thread_supression_noninterference")
4247+
4248+
# Block on the first call, allow others through
4249+
#
4250+
# NOTE: We need to bypass the base class's lock, otherwise that will
4251+
# block multiple calls to the same handler itself.
4252+
class BlockOnceHandler(TestHandler):
4253+
def __init__(self, barrier):
4254+
super().__init__(support.Matcher())
4255+
self.barrier = barrier
4256+
4257+
def createLock(self):
4258+
self.lock = None
4259+
4260+
def handle(self, record):
4261+
self.emit(record)
4262+
4263+
def emit(self, record):
4264+
if self.barrier:
4265+
barrier = self.barrier
4266+
self.barrier = None
4267+
barrier.wait()
4268+
with lock:
4269+
pass
4270+
super().emit(record)
4271+
logger.info("blow up if not supressed")
4272+
4273+
barrier = threading.Barrier(2)
4274+
handler = BlockOnceHandler(barrier)
4275+
logger.addHandler(handler)
4276+
logger.setLevel(logging.DEBUG)
4277+
4278+
t1 = threading.Thread(target=logger.debug, args=("1",))
4279+
with lock:
4280+
4281+
# Ensure first thread is blocked in the handler, hence supressing logging...
4282+
t1.start()
4283+
barrier.wait()
4284+
4285+
# ...but the second thread should still be able to log...
4286+
t2 = threading.Thread(target=logger.debug, args=("2",))
4287+
t2.start()
4288+
t2.join(timeout=3)
4289+
4290+
self.assertEqual(len(handler.buffer), 1)
4291+
self.assertTrue(handler.matches(levelno=logging.DEBUG, message='2'))
4292+
4293+
# The first thread should still be blocked here
4294+
self.assertTrue(t1.is_alive())
4295+
4296+
# Now the lock has been released the first thread should complete
4297+
t1.join()
4298+
self.assertEqual(len(handler.buffer), 2)
4299+
self.assertTrue(handler.matches(levelno=logging.DEBUG, message='1'))
42174300

42184301
class ManagerTest(BaseTest):
42194302
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)