From 5046e40d17852866071984c1da451eca1bb69b84 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 10 Aug 2017 13:20:13 +0200 Subject: [PATCH 1/2] Add socketserver.ForkingMixIn.server_close() bpo-31151: socketserver.ForkingMixIn.server_close() now waits until all child processes completed to prevent leaking zombie processes. --- Lib/socketserver.py | 9 +++++++-- Lib/test/test_socketserver.py | 9 +++++---- .../Library/2017-08-10-13-20-02.bpo-31151.730VBI.rst | 2 ++ 3 files changed, 14 insertions(+), 6 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2017-08-10-13-20-02.bpo-31151.730VBI.rst diff --git a/Lib/socketserver.py b/Lib/socketserver.py index 6e1ae9fddda1116..df1711495011362 100644 --- a/Lib/socketserver.py +++ b/Lib/socketserver.py @@ -547,7 +547,7 @@ class ForkingMixIn: active_children = None max_children = 40 - def collect_children(self): + def collect_children(self, *, blocking=False): """Internal routine to wait for children that have exited.""" if self.active_children is None: return @@ -571,7 +571,8 @@ def collect_children(self): # Now reap all defunct children. for pid in self.active_children.copy(): try: - pid, _ = os.waitpid(pid, os.WNOHANG) + flags = 0 if blocking else os.WNOHANG + pid, _ = os.waitpid(pid, flags) # if the child hasn't exited yet, pid will be 0 and ignored by # discard() below self.active_children.discard(pid) @@ -620,6 +621,10 @@ def process_request(self, request, client_address): finally: os._exit(status) + def server_close(self): + super().server_close() + self.collect_children(blocking=True) + class ThreadingMixIn: """Mix-in class to handle each request in a new thread.""" diff --git a/Lib/test/test_socketserver.py b/Lib/test/test_socketserver.py index 140a6abf9efb9ce..9828b5ef0ccac6f 100644 --- a/Lib/test/test_socketserver.py +++ b/Lib/test/test_socketserver.py @@ -144,6 +144,10 @@ def run_server(self, svrcls, hdlrbase, testfunc): t.join() server.server_close() self.assertEqual(-1, server.socket.fileno()) + if isinstance(server, socketserver.ForkingMixIn): + # bpo-31151: Check that ForkingMixIn.server_close() waits until + # all children completed + self.assertFalse(server.active_children) if verbose: print("done") def stream_examine(self, proto, addr): @@ -371,10 +375,7 @@ def wait_done(self): if HAVE_FORKING: class ForkingErrorTestServer(socketserver.ForkingMixIn, BaseErrorTestServer): - def wait_done(self): - [child] = self.active_children - os.waitpid(child, 0) - self.active_children.clear() + pass class SocketWriterTest(unittest.TestCase): diff --git a/Misc/NEWS.d/next/Library/2017-08-10-13-20-02.bpo-31151.730VBI.rst b/Misc/NEWS.d/next/Library/2017-08-10-13-20-02.bpo-31151.730VBI.rst new file mode 100644 index 000000000000000..6eff63653ab3db8 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2017-08-10-13-20-02.bpo-31151.730VBI.rst @@ -0,0 +1,2 @@ +socketserver.ForkingMixIn.server_close() now waits until all child processes +completed to prevent leaking zombie processes. From 28809da4d0fa9c66d641e57450849c05612dc7ec Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 10 Aug 2017 14:13:23 +0200 Subject: [PATCH 2/2] Fix test on Windows which doesn't have ForkingMixIn --- Lib/test/test_socketserver.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_socketserver.py b/Lib/test/test_socketserver.py index 9828b5ef0ccac6f..3d93566607d31b1 100644 --- a/Lib/test/test_socketserver.py +++ b/Lib/test/test_socketserver.py @@ -144,7 +144,7 @@ def run_server(self, svrcls, hdlrbase, testfunc): t.join() server.server_close() self.assertEqual(-1, server.socket.fileno()) - if isinstance(server, socketserver.ForkingMixIn): + if HAVE_FORKING and isinstance(server, socketserver.ForkingMixIn): # bpo-31151: Check that ForkingMixIn.server_close() waits until # all children completed self.assertFalse(server.active_children)