From 77e17727617b50072aac55f6fc592e3afc7f6a6b Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 13 Sep 2017 02:13:16 +0200 Subject: [PATCH 1/4] bpo-31233: socketserver.ThreadingMixIn.server_close() socketserver.ThreadingMixIn now keeps a list of non-daemonic threads to wait until all these threads complete in server_close(). --- Lib/socketserver.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Lib/socketserver.py b/Lib/socketserver.py index 721eb50fbe683a..1ae7bef9040723 100644 --- a/Lib/socketserver.py +++ b/Lib/socketserver.py @@ -629,6 +629,9 @@ class ThreadingMixIn: # Decides how threads will act upon termination of the # main process daemon_threads = False + # For non-daemonic threads, list of threading.Threading objects + # used by server_close() to wait for all threads completion. + _threads = None def process_request_thread(self, request, client_address): """Same as in BaseServer but as a thread. @@ -648,8 +651,20 @@ def process_request(self, request, client_address): t = threading.Thread(target = self.process_request_thread, args = (request, client_address)) t.daemon = self.daemon_threads + if not t.daemon: + if self._threads is None: + self._threads = [] + self._threads.append(t) t.start() + def server_close(self): + super().server_close() + threads = self._threads + self._threads = None + if threads: + for thread in threads: + thread.join() + if hasattr(os, "fork"): class ForkingUDPServer(ForkingMixIn, UDPServer): pass From eb8f1f0cdfcf144541e0830d49c0836bae841d78 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 13 Sep 2017 02:17:17 +0200 Subject: [PATCH 2/4] Add NEWS entry. --- .../next/Library/2017-09-13-02-17-11.bpo-31233.r-IPIu.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2017-09-13-02-17-11.bpo-31233.r-IPIu.rst diff --git a/Misc/NEWS.d/next/Library/2017-09-13-02-17-11.bpo-31233.r-IPIu.rst b/Misc/NEWS.d/next/Library/2017-09-13-02-17-11.bpo-31233.r-IPIu.rst new file mode 100644 index 00000000000000..5cf75e7c9f8d63 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2017-09-13-02-17-11.bpo-31233.r-IPIu.rst @@ -0,0 +1,2 @@ +socketserver.ThreadingMixIn now keeps a list of non-daemonic threads to wait +until all these threads complete in server_close(). From a317d50bf97b3e1584cb0e490b4ff57618456fee Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 13 Sep 2017 02:36:54 +0200 Subject: [PATCH 3/4] Reenable test_logging skipped tests Fix SocketHandlerTest.tearDown(): close the socket handler before stopping the server, so the server can join threads. --- Lib/test/test_logging.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/Lib/test/test_logging.py b/Lib/test/test_logging.py index 9c3816ada770bb..0cf3236095f664 100644 --- a/Lib/test/test_logging.py +++ b/Lib/test/test_logging.py @@ -1465,7 +1465,6 @@ def test_logger_disabling(self): self.assertFalse(logger.disabled) -@unittest.skipIf(True, "FIXME: bpo-30830") class SocketHandlerTest(BaseTest): """Test for SocketHandler objects.""" @@ -1502,11 +1501,11 @@ def setUp(self): def tearDown(self): """Shutdown the TCP server.""" try: - if self.server: - self.server.stop(2.0) if self.sock_hdlr: self.root_logger.removeHandler(self.sock_hdlr) self.sock_hdlr.close() + if self.server: + self.server.stop(2.0) finally: BaseTest.tearDown(self) @@ -1563,7 +1562,6 @@ def _get_temp_domain_socket(): os.remove(fn) return fn -@unittest.skipIf(True, "FIXME: bpo-30830") @unittest.skipUnless(hasattr(socket, "AF_UNIX"), "Unix sockets required") class UnixSocketHandlerTest(SocketHandlerTest): @@ -1581,7 +1579,6 @@ def tearDown(self): SocketHandlerTest.tearDown(self) support.unlink(self.address) -@unittest.skipIf(True, "FIXME: bpo-30830") class DatagramHandlerTest(BaseTest): """Test for DatagramHandler.""" @@ -1646,7 +1643,6 @@ def test_output(self): self.handled.wait() self.assertEqual(self.log_output, "spam\neggs\n") -@unittest.skipIf(True, "FIXME: bpo-30830") @unittest.skipUnless(hasattr(socket, "AF_UNIX"), "Unix sockets required") class UnixDatagramHandlerTest(DatagramHandlerTest): @@ -1731,7 +1727,6 @@ def test_output(self): self.handled.wait() self.assertEqual(self.log_output, b'<11>h\xc3\xa4m-sp\xc3\xa4m') -@unittest.skipIf(True, "FIXME: bpo-30830") @unittest.skipUnless(hasattr(socket, "AF_UNIX"), "Unix sockets required") class UnixSysLogHandlerTest(SysLogHandlerTest): @@ -1749,7 +1744,6 @@ def tearDown(self): SysLogHandlerTest.tearDown(self) support.unlink(self.address) -@unittest.skipIf(True, "FIXME: bpo-30830") @unittest.skipUnless(support.IPV6_ENABLED, 'IPv6 support required for this test.') class IPv6SysLogHandlerTest(SysLogHandlerTest): @@ -2874,7 +2868,6 @@ def test_config14_ok(self): # listen() uses ConfigSocketReceiver which is based # on socketserver.ThreadingTCPServer - @unittest.skipIf(True, "FIXME: bpo-30830") def setup_via_listener(self, text, verify=None): text = text.encode("utf-8") # Ask for a randomly assigned port (by using port 0) From 7c0ef3ffb4baead2a8442a79b73930258e4f57ce Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Wed, 13 Sep 2017 03:03:14 +0200 Subject: [PATCH 4/4] Remove outdated comment --- Lib/test/test_logging.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Lib/test/test_logging.py b/Lib/test/test_logging.py index 0cf3236095f664..76f98bb572d6e6 100644 --- a/Lib/test/test_logging.py +++ b/Lib/test/test_logging.py @@ -2866,8 +2866,6 @@ def test_config14_ok(self): logging.warning('Exclamation') self.assertTrue(output.getvalue().endswith('Exclamation!\n')) - # listen() uses ConfigSocketReceiver which is based - # on socketserver.ThreadingTCPServer def setup_via_listener(self, text, verify=None): text = text.encode("utf-8") # Ask for a randomly assigned port (by using port 0)