From aa8ec34ad52bb3b274ce91169e1bc4a598655049 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 10 Aug 2017 15:28:16 +0200 Subject: bpo-31151: Add socketserver.ForkingMixIn.server_close() (#3057) * Add socketserver.ForkingMixIn.server_close() bpo-31151: socketserver.ForkingMixIn.server_close() now waits until all child processes completed to prevent leaking zombie processes. * Fix test on Windows which doesn't have ForkingMixIn --- Lib/socketserver.py | 9 +++++++-- Lib/test/test_socketserver.py | 9 +++++---- .../NEWS.d/next/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 6e1ae9f..df17114 100644 --- a/Lib/socketserver.py +++ b/Lib/socketserver.py @@ -547,7 +547,7 @@ if hasattr(os, "fork"): 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 @@ if hasattr(os, "fork"): # 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 @@ if hasattr(os, "fork"): 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 140a6ab..3d93566 100644 --- a/Lib/test/test_socketserver.py +++ b/Lib/test/test_socketserver.py @@ -144,6 +144,10 @@ class SocketServerTest(unittest.TestCase): t.join() server.server_close() self.assertEqual(-1, server.socket.fileno()) + 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) if verbose: print("done") def stream_examine(self, proto, addr): @@ -371,10 +375,7 @@ class ThreadingErrorTestServer(socketserver.ThreadingMixIn, 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 0000000..6eff636 --- /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. -- cgit v0.12