Skip to content

Commit bc237ed

Browse files
[3.12] gh-124653: Relax (again) detection of queue API for logging handlers (GH-124897) (GH-125060)
(cherry picked from commit 7ffe94f)
1 parent 79b4c9d commit bc237ed

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
@@ -752,16 +752,17 @@ The ``queue`` and ``listener`` keys are optional.
752752

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

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

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

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

767768
* 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
@@ -502,7 +502,7 @@ def as_tuple(self, value):
502502

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

530530
class DictConfigurator(BaseConfigurator):
531531
"""

Lib/test/test_logging.py

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

23932393
class CustomQueueFakeProtocol(CustomQueueProtocol):
2394-
# An object implementing the Queue API (incorrect signatures).
2394+
# An object implementing the minimial Queue API for
2395+
# the logging module but with incorrect signatures.
2396+
#
23952397
# The object will be considered a valid queue class since we
23962398
# do not check the signatures (only callability of methods)
23972399
# but will NOT be usable in production since a TypeError will
2398-
# be raised due to a missing argument.
2399-
def empty(self, x):
2400+
# be raised due to the extra argument in 'put_nowait'.
2401+
def put_nowait(self):
24002402
pass
24012403

24022404
class CustomQueueWrongProtocol(CustomQueueProtocol):
2403-
empty = None
2405+
put_nowait = None
2406+
2407+
class MinimalQueueProtocol:
2408+
def put_nowait(self, x): pass
2409+
def get(self): pass
24042410

24052411
def queueMaker():
24062412
return queue.Queue()
@@ -3914,56 +3920,70 @@ def test_config_queue_handler(self):
39143920
msg = str(ctx.exception)
39153921
self.assertEqual(msg, "Unable to configure handler 'ah'")
39163922

3923+
def _apply_simple_queue_listener_configuration(self, qspec):
3924+
self.apply_config({
3925+
"version": 1,
3926+
"handlers": {
3927+
"queue_listener": {
3928+
"class": "logging.handlers.QueueHandler",
3929+
"queue": qspec,
3930+
},
3931+
},
3932+
})
3933+
39173934
@threading_helper.requires_working_threading()
39183935
@support.requires_subprocess()
39193936
@patch("multiprocessing.Manager")
39203937
def test_config_queue_handler_does_not_create_multiprocessing_manager(self, manager):
3921-
# gh-120868, gh-121723
3922-
3923-
from multiprocessing import Queue as MQ
3924-
3925-
q1 = {"()": "queue.Queue", "maxsize": -1}
3926-
q2 = MQ()
3927-
q3 = queue.Queue()
3928-
# CustomQueueFakeProtocol passes the checks but will not be usable
3929-
# since the signatures are incompatible. Checking the Queue API
3930-
# without testing the type of the actual queue is a trade-off
3931-
# between usability and the work we need to do in order to safely
3932-
# check that the queue object correctly implements the API.
3933-
q4 = CustomQueueFakeProtocol()
3934-
3935-
for qspec in (q1, q2, q3, q4):
3936-
self.apply_config(
3937-
{
3938-
"version": 1,
3939-
"handlers": {
3940-
"queue_listener": {
3941-
"class": "logging.handlers.QueueHandler",
3942-
"queue": qspec,
3943-
},
3944-
},
3945-
}
3946-
)
3947-
manager.assert_not_called()
3938+
# gh-120868, gh-121723, gh-124653
3939+
3940+
for qspec in [
3941+
{"()": "queue.Queue", "maxsize": -1},
3942+
queue.Queue(),
3943+
# queue.SimpleQueue does not inherit from queue.Queue
3944+
queue.SimpleQueue(),
3945+
# CustomQueueFakeProtocol passes the checks but will not be usable
3946+
# since the signatures are incompatible. Checking the Queue API
3947+
# without testing the type of the actual queue is a trade-off
3948+
# between usability and the work we need to do in order to safely
3949+
# check that the queue object correctly implements the API.
3950+
CustomQueueFakeProtocol(),
3951+
MinimalQueueProtocol(),
3952+
]:
3953+
with self.subTest(qspec=qspec):
3954+
self._apply_simple_queue_listener_configuration(qspec)
3955+
manager.assert_not_called()
39483956

39493957
@patch("multiprocessing.Manager")
39503958
def test_config_queue_handler_invalid_config_does_not_create_multiprocessing_manager(self, manager):
39513959
# gh-120868, gh-121723
39523960

39533961
for qspec in [object(), CustomQueueWrongProtocol()]:
3954-
with self.assertRaises(ValueError):
3955-
self.apply_config(
3956-
{
3957-
"version": 1,
3958-
"handlers": {
3959-
"queue_listener": {
3960-
"class": "logging.handlers.QueueHandler",
3961-
"queue": qspec,
3962-
},
3963-
},
3964-
}
3965-
)
3966-
manager.assert_not_called()
3962+
with self.subTest(qspec=qspec), self.assertRaises(ValueError):
3963+
self._apply_simple_queue_listener_configuration(qspec)
3964+
manager.assert_not_called()
3965+
3966+
@skip_if_tsan_fork
3967+
@support.requires_subprocess()
3968+
@unittest.skipUnless(support.Py_DEBUG, "requires a debug build for testing"
3969+
" assertions in multiprocessing")
3970+
def test_config_reject_simple_queue_handler_multiprocessing_context(self):
3971+
# multiprocessing.SimpleQueue does not implement 'put_nowait'
3972+
# and thus cannot be used as a queue-like object (gh-124653)
3973+
3974+
import multiprocessing
3975+
3976+
if support.MS_WINDOWS:
3977+
start_methods = ['spawn']
3978+
else:
3979+
start_methods = ['spawn', 'fork', 'forkserver']
3980+
3981+
for start_method in start_methods:
3982+
with self.subTest(start_method=start_method):
3983+
ctx = multiprocessing.get_context(start_method)
3984+
qspec = ctx.SimpleQueue()
3985+
with self.assertRaises(ValueError):
3986+
self._apply_simple_queue_listener_configuration(qspec)
39673987

39683988
@skip_if_tsan_fork
39693989
@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)