Skip to content

Commit 1e820e6

Browse files
[3.13] gh-124653: Relax (again) detection of queue API for logging handlers (GH-124897) (GH-125059)
(cherry picked from commit 7ffe94f)
1 parent 761c3b2 commit 1e820e6

File tree

4 files changed

+79
-56
lines changed

4 files changed

+79
-56
lines changed

Doc/library/logging.config.rst

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -753,16 +753,17 @@ The ``queue`` and ``listener`` keys are optional.
753753

754754
If the ``queue`` key is present, the corresponding value can be one of the following:
755755

756-
* An object implementing the :class:`queue.Queue` public API. For instance,
757-
this may be an actual instance of :class:`queue.Queue` or a subclass thereof,
758-
or a proxy obtained by :meth:`multiprocessing.managers.SyncManager.Queue`.
756+
* An object implementing the :meth:`Queue.put_nowait <queue.Queue.put_nowait>`
757+
and :meth:`Queue.get <queue.Queue.get>` public API. For instance, this may be
758+
an actual instance of :class:`queue.Queue` or a subclass thereof, or a proxy
759+
obtained by :meth:`multiprocessing.managers.SyncManager.Queue`.
759760

760761
This is of course only possible if you are constructing or modifying
761762
the configuration dictionary in code.
762763

763764
* A string that resolves to a callable which, when called with no arguments, returns
764-
the :class:`queue.Queue` instance to use. That callable could be a
765-
:class:`queue.Queue` subclass or a function which returns a suitable queue instance,
765+
the queue instance to use. That callable could be a :class:`queue.Queue` subclass
766+
or a function which returns a suitable queue instance,
766767
such as ``my.module.queue_factory()``.
767768

768769
* A dict with a ``'()'`` key which is constructed in the usual way as discussed in

Lib/logging/config.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,7 @@ def as_tuple(self, value):
499499

500500
def _is_queue_like_object(obj):
501501
"""Check that *obj* implements the Queue API."""
502-
if isinstance(obj, queue.Queue):
502+
if isinstance(obj, (queue.Queue, queue.SimpleQueue)):
503503
return True
504504
# defer importing multiprocessing as much as possible
505505
from multiprocessing.queues import Queue as MPQueue
@@ -516,13 +516,13 @@ def _is_queue_like_object(obj):
516516
# Ideally, we would have wanted to simply use strict type checking
517517
# instead of a protocol-based type checking since the latter does
518518
# not check the method signatures.
519-
queue_interface = [
520-
'empty', 'full', 'get', 'get_nowait',
521-
'put', 'put_nowait', 'join', 'qsize',
522-
'task_done',
523-
]
519+
#
520+
# Note that only 'put_nowait' and 'get' are required by the logging
521+
# queue handler and queue listener (see gh-124653) and that other
522+
# methods are either optional or unused.
523+
minimal_queue_interface = ['put_nowait', 'get']
524524
return all(callable(getattr(obj, method, None))
525-
for method in queue_interface)
525+
for method in minimal_queue_interface)
526526

527527
class DictConfigurator(BaseConfigurator):
528528
"""

Lib/test/test_logging.py

Lines changed: 64 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -2376,16 +2376,22 @@ def __getattr__(self, attribute):
23762376
return getattr(queue, attribute)
23772377

23782378
class CustomQueueFakeProtocol(CustomQueueProtocol):
2379-
# An object implementing the Queue API (incorrect signatures).
2379+
# An object implementing the minimial Queue API for
2380+
# the logging module but with incorrect signatures.
2381+
#
23802382
# The object will be considered a valid queue class since we
23812383
# do not check the signatures (only callability of methods)
23822384
# but will NOT be usable in production since a TypeError will
2383-
# be raised due to a missing argument.
2384-
def empty(self, x):
2385+
# be raised due to the extra argument in 'put_nowait'.
2386+
def put_nowait(self):
23852387
pass
23862388

23872389
class CustomQueueWrongProtocol(CustomQueueProtocol):
2388-
empty = None
2390+
put_nowait = None
2391+
2392+
class MinimalQueueProtocol:
2393+
def put_nowait(self, x): pass
2394+
def get(self): pass
23892395

23902396
def queueMaker():
23912397
return queue.Queue()
@@ -3945,56 +3951,70 @@ def test_config_queue_handler(self):
39453951
msg = str(ctx.exception)
39463952
self.assertEqual(msg, "Unable to configure handler 'ah'")
39473953

3954+
def _apply_simple_queue_listener_configuration(self, qspec):
3955+
self.apply_config({
3956+
"version": 1,
3957+
"handlers": {
3958+
"queue_listener": {
3959+
"class": "logging.handlers.QueueHandler",
3960+
"queue": qspec,
3961+
},
3962+
},
3963+
})
3964+
39483965
@threading_helper.requires_working_threading()
39493966
@support.requires_subprocess()
39503967
@patch("multiprocessing.Manager")
39513968
def test_config_queue_handler_does_not_create_multiprocessing_manager(self, manager):
3952-
# gh-120868, gh-121723
3953-
3954-
from multiprocessing import Queue as MQ
3955-
3956-
q1 = {"()": "queue.Queue", "maxsize": -1}
3957-
q2 = MQ()
3958-
q3 = queue.Queue()
3959-
# CustomQueueFakeProtocol passes the checks but will not be usable
3960-
# since the signatures are incompatible. Checking the Queue API
3961-
# without testing the type of the actual queue is a trade-off
3962-
# between usability and the work we need to do in order to safely
3963-
# check that the queue object correctly implements the API.
3964-
q4 = CustomQueueFakeProtocol()
3965-
3966-
for qspec in (q1, q2, q3, q4):
3967-
self.apply_config(
3968-
{
3969-
"version": 1,
3970-
"handlers": {
3971-
"queue_listener": {
3972-
"class": "logging.handlers.QueueHandler",
3973-
"queue": qspec,
3974-
},
3975-
},
3976-
}
3977-
)
3978-
manager.assert_not_called()
3969+
# gh-120868, gh-121723, gh-124653
3970+
3971+
for qspec in [
3972+
{"()": "queue.Queue", "maxsize": -1},
3973+
queue.Queue(),
3974+
# queue.SimpleQueue does not inherit from queue.Queue
3975+
queue.SimpleQueue(),
3976+
# CustomQueueFakeProtocol passes the checks but will not be usable
3977+
# since the signatures are incompatible. Checking the Queue API
3978+
# without testing the type of the actual queue is a trade-off
3979+
# between usability and the work we need to do in order to safely
3980+
# check that the queue object correctly implements the API.
3981+
CustomQueueFakeProtocol(),
3982+
MinimalQueueProtocol(),
3983+
]:
3984+
with self.subTest(qspec=qspec):
3985+
self._apply_simple_queue_listener_configuration(qspec)
3986+
manager.assert_not_called()
39793987

39803988
@patch("multiprocessing.Manager")
39813989
def test_config_queue_handler_invalid_config_does_not_create_multiprocessing_manager(self, manager):
39823990
# gh-120868, gh-121723
39833991

39843992
for qspec in [object(), CustomQueueWrongProtocol()]:
3985-
with self.assertRaises(ValueError):
3986-
self.apply_config(
3987-
{
3988-
"version": 1,
3989-
"handlers": {
3990-
"queue_listener": {
3991-
"class": "logging.handlers.QueueHandler",
3992-
"queue": qspec,
3993-
},
3994-
},
3995-
}
3996-
)
3997-
manager.assert_not_called()
3993+
with self.subTest(qspec=qspec), self.assertRaises(ValueError):
3994+
self._apply_simple_queue_listener_configuration(qspec)
3995+
manager.assert_not_called()
3996+
3997+
@skip_if_tsan_fork
3998+
@support.requires_subprocess()
3999+
@unittest.skipUnless(support.Py_DEBUG, "requires a debug build for testing"
4000+
" assertions in multiprocessing")
4001+
def test_config_reject_simple_queue_handler_multiprocessing_context(self):
4002+
# multiprocessing.SimpleQueue does not implement 'put_nowait'
4003+
# and thus cannot be used as a queue-like object (gh-124653)
4004+
4005+
import multiprocessing
4006+
4007+
if support.MS_WINDOWS:
4008+
start_methods = ['spawn']
4009+
else:
4010+
start_methods = ['spawn', 'fork', 'forkserver']
4011+
4012+
for start_method in start_methods:
4013+
with self.subTest(start_method=start_method):
4014+
ctx = multiprocessing.get_context(start_method)
4015+
qspec = ctx.SimpleQueue()
4016+
with self.assertRaises(ValueError):
4017+
self._apply_simple_queue_listener_configuration(qspec)
39984018

39994019
@skip_if_tsan_fork
40004020
@support.requires_subprocess()
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix detection of the minimal Queue API needed by the :mod:`logging` module.
2+
Patch by Bénédikt Tran.

0 commit comments

Comments
 (0)