[Python-checkins] bpo-31233: socketserver.ThreadingMixIn.server_close() (#3523)

Wed Sep 13 04:47:25 EDT 2017

https://github.com/python/cpython/commit/b8f4163da30e16c7cd58fe04f4b17e38d53cd57e
commit: b8f4163da30e16c7cd58fe04f4b17e38d53cd57e
branch: master
author: Victor Stinner <victor.stinner at gmail.com>
committer: GitHub <noreply at github.com>
date: 2017年09月13日T01:47:22-07:00
summary:
bpo-31233: socketserver.ThreadingMixIn.server_close() (#3523)
socketserver.ThreadingMixIn now keeps a list of non-daemonic threads
to wait until all these threads complete in server_close().
Reenable test_logging skipped tests.
Fix SocketHandlerTest.tearDown(): close the socket handler before
stopping the server, so the server can join threads.
files:
A Misc/NEWS.d/next/Library/2017-09-13-02-17-11.bpo-31233.r-IPIu.rst
M Lib/socketserver.py
M Lib/test/test_logging.py

diff --git a/Lib/socketserver.py b/Lib/socketserver.py
index 721eb50fbe6..1ae7bef9040 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
diff --git a/Lib/test/test_logging.py b/Lib/test/test_logging.py
index 9c3816ada77..76f98bb572d 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)
 
 
- at 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
 
- at 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)
 
- at 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")
 
- at 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')
 
- at 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)
 
- at unittest.skipIf(True, "FIXME: bpo-30830")
 @unittest.skipUnless(support.IPV6_ENABLED,
 'IPv6 support required for this test.')
 class IPv6SysLogHandlerTest(SysLogHandlerTest):
@@ -2872,9 +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
- @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)
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 00000000000..5cf75e7c9f8
--- /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().
