pythongh-120868: Fix breaking change in logging.config when using `…

…QueueHandler` (pythonGH-120872)
estyxx · Jul 17, 2024 · e9dd992 · e9dd992
1 parent 1a151b0
commit e9dd992
Show file tree

Hide file tree

Showing 3 changed files with 81 additions and 17 deletions.
diff --git a/Lib/logging/config.py b/Lib/logging/config.py
@@ -780,25 +780,44 @@ def configure_handler(self, config):
                 # if 'handlers' not in config:
                     # raise ValueError('No handlers specified for a QueueHandler')
                 if 'queue' in config:
-                    from multiprocessing.queues import Queue as MPQueue
-                    from multiprocessing import Manager as MM
-                    proxy_queue = MM().Queue()
-                    proxy_joinable_queue = MM().JoinableQueue()
                     qspec = config['queue']
-                    if not isinstance(qspec, (queue.Queue, MPQueue,
-                                      type(proxy_queue), type(proxy_joinable_queue))):
-                        if isinstance(qspec, str):
-                            q = self.resolve(qspec)
-                            if not callable(q):
-                                raise TypeError('Invalid queue specifier %r' % qspec)
-                            q = q()
-                        elif isinstance(qspec, dict):
-                            if '()' not in qspec:
-                                raise TypeError('Invalid queue specifier %r' % qspec)
-                            q = self.configure_custom(dict(qspec))
-                        else:
+
+                    if isinstance(qspec, str):
+                        q = self.resolve(qspec)
+                        if not callable(q):
                             raise TypeError('Invalid queue specifier %r' % qspec)
-                        config['queue'] = q
+                        config['queue'] = q()
+                    elif isinstance(qspec, dict):
+                        if '()' not in qspec:
+                            raise TypeError('Invalid queue specifier %r' % qspec)
+                        config['queue'] = self.configure_custom(dict(qspec))
+                    else:
+                        from multiprocessing.queues import Queue as MPQueue
+
+                        if not isinstance(qspec, (queue.Queue, MPQueue)):
+                            # Safely check if 'qspec' is an instance of Manager.Queue
+                            # / Manager.JoinableQueue
+
+                            from multiprocessing import Manager as MM
+                            from multiprocessing.managers import BaseProxy
+
+                            # if it's not an instance of BaseProxy, it also can't be
+                            # an instance of Manager.Queue / Manager.JoinableQueue
+                            if isinstance(qspec, BaseProxy):
+                                # Sometimes manager or queue creation might fail
+                                # (e.g. see issue gh-120868). In that case, any
+                                # exception during the creation of these queues will
+                                # propagate up to the caller and be wrapped in a
+                                # `ValueError`, whose cause will indicate the details of
+                                # the failure.
+                                mm = MM()
+                                proxy_queue = mm.Queue()
+                                proxy_joinable_queue = mm.JoinableQueue()
+                                if not isinstance(qspec, (type(proxy_queue), type(proxy_joinable_queue))):
+                                    raise TypeError('Invalid queue specifier %r' % qspec)
+                            else:
+                                raise TypeError('Invalid queue specifier %r' % qspec)
+
                 if 'listener' in config:
                     lspec = config['listener']
                     if isinstance(lspec, type):

diff --git a/Lib/test/test_logging.py b/Lib/test/test_logging.py
@@ -3928,6 +3928,50 @@ def test_config_queue_handler(self):
             msg = str(ctx.exception)
             self.assertEqual(msg, "Unable to configure handler 'ah'")
 
+    @threading_helper.requires_working_threading()
+    @support.requires_subprocess()
+    @patch("multiprocessing.Manager")
+    def test_config_queue_handler_does_not_create_multiprocessing_manager(self, manager):
+        # gh-120868
+
+        from multiprocessing import Queue as MQ
+
+        q1 = {"()": "queue.Queue", "maxsize": -1}
+        q2 = MQ()
+        q3 = queue.Queue()
+
+        for qspec in (q1, q2, q3):
+            self.apply_config(
+                {
+                    "version": 1,
+                    "handlers": {
+                        "queue_listener": {
+                            "class": "logging.handlers.QueueHandler",
+                            "queue": qspec,
+                        },
+                    },
+                }
+            )
+            manager.assert_not_called()
+
+    @patch("multiprocessing.Manager")
+    def test_config_queue_handler_invalid_config_does_not_create_multiprocessing_manager(self, manager):
+        # gh-120868
+
+        with self.assertRaises(ValueError):
+            self.apply_config(
+                {
+                    "version": 1,
+                    "handlers": {
+                        "queue_listener": {
+                            "class": "logging.handlers.QueueHandler",
+                            "queue": object(),
+                        },
+                    },
+                }
+            )
+        manager.assert_not_called()
+
     @support.requires_subprocess()
     def test_multiprocessing_queues(self):
         # See gh-119819

diff --git a/Misc/ACKS b/Misc/ACKS
@@ -1318,6 +1318,7 @@ Hrvoje Nikšić
 Gregory Nofi
 Jesse Noller
 Bill Noon
+Janek Nouvertné
 Stefan Norberg
 Tim Northover
 Joe Norton