Skip to content

bpo-31233: socketserver.ThreadingMixIn.server_close() #3523

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 13, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions Lib/socketserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
13 changes: 2 additions & 11 deletions Lib/test/test_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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):

Expand All @@ -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."""
Expand Down Expand Up @@ -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):

Expand Down Expand Up @@ -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):

Expand All @@ -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):
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
socketserver.ThreadingMixIn now keeps a list of non-daemonic threads to wait
until all these threads complete in server_close().