[Python-checkins] [3.6] bpo-31308: If multiprocessing's forkserver dies, launch it again when necessary (GH-3246) (#4252)
Antoine Pitrou
webhook-mailer at python.org
Fri Nov 3 08:59:49 EDT 2017
https://github.com/python/cpython/commit/019c99f325287741d1e0eefeef2b75c8e00b884f
commit: 019c99f325287741d1e0eefeef2b75c8e00b884f
branch: 3.6
author: Antoine Pitrou <pitrou at free.fr>
committer: GitHub <noreply at github.com>
date: 2017-11-03T13:59:43+01:00
summary:
[3.6] bpo-31308: If multiprocessing's forkserver dies, launch it again when necessary (GH-3246) (#4252)
* bpo-31308: If multiprocessing's forkserver dies, launch it again when necessary.
* Fix test on Windows
* Add NEWS entry
* Adopt a different approach: ignore SIGINT and SIGTERM, as in semaphore tracker.
* Fix comment
* Make sure the test doesn't muck with process state
* Also test previously-started processes
* Update 2017-08-30-17-59-36.bpo-31308.KbexyC.rst
* Avoid masking SIGTERM in forkserver. It's not necessary and causes a race condition in test_many_processes..
(cherry picked from commit fc6b348b12ad401cab0261b7b71a65c60a08c0a8)
files:
A Misc/NEWS.d/next/Library/2017-08-30-17-59-36.bpo-31308.KbexyC.rst
M Lib/multiprocessing/forkserver.py
M Lib/test/_test_multiprocessing.py
diff --git a/Lib/multiprocessing/forkserver.py b/Lib/multiprocessing/forkserver.py
index d5ce6257456..fd198959c8a 100644
--- a/Lib/multiprocessing/forkserver.py
+++ b/Lib/multiprocessing/forkserver.py
@@ -33,6 +33,7 @@ class ForkServer(object):
def __init__(self):
self._forkserver_address = None
self._forkserver_alive_fd = None
+ self._forkserver_pid = None
self._inherited_fds = None
self._lock = threading.Lock()
self._preload_modules = ['__main__']
@@ -89,8 +90,17 @@ def ensure_running(self):
'''
with self._lock:
semaphore_tracker.ensure_running()
- if self._forkserver_alive_fd is not None:
- return
+ if self._forkserver_pid is not None:
+ # forkserver was launched before, is it still running?
+ pid, status = os.waitpid(self._forkserver_pid, os.WNOHANG)
+ if not pid:
+ # still alive
+ return
+ # dead, launch it again
+ os.close(self._forkserver_alive_fd)
+ self._forkserver_address = None
+ self._forkserver_alive_fd = None
+ self._forkserver_pid = None
cmd = ('from multiprocessing.forkserver import main; ' +
'main(%d, %d, %r, **%r)')
@@ -127,6 +137,7 @@ def ensure_running(self):
os.close(alive_r)
self._forkserver_address = address
self._forkserver_alive_fd = alive_w
+ self._forkserver_pid = pid
#
#
@@ -149,11 +160,11 @@ def main(listener_fd, alive_r, preload, main_path=None, sys_path=None):
util._close_stdin()
- # ignoring SIGCHLD means no need to reap zombie processes;
- # letting SIGINT through avoids KeyboardInterrupt tracebacks
handlers = {
+ # no need to reap zombie processes;
signal.SIGCHLD: signal.SIG_IGN,
- signal.SIGINT: signal.SIG_DFL,
+ # protect the process from ^C
+ signal.SIGINT: signal.SIG_IGN,
}
old_handlers = {sig: signal.signal(sig, val)
for (sig, val) in handlers.items()}
diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py
index 80c95d6bca6..056474baa44 100644
--- a/Lib/test/_test_multiprocessing.py
+++ b/Lib/test/_test_multiprocessing.py
@@ -446,6 +446,54 @@ def test_error_on_stdio_flush(self):
finally:
setattr(sys, stream_name, old_stream)
+ @classmethod
+ def _sleep_and_set_event(self, evt, delay=0.0):
+ time.sleep(delay)
+ evt.set()
+
+ def check_forkserver_death(self, signum):
+ # bpo-31308: if the forkserver process has died, we should still
+ # be able to create and run new Process instances (the forkserver
+ # is implicitly restarted).
+ if self.TYPE == 'threads':
+ self.skipTest('test not appropriate for {}'.format(self.TYPE))
+ sm = multiprocessing.get_start_method()
+ if sm != 'forkserver':
+ # The fork method by design inherits all fds from the parent,
+ # trying to go against it is a lost battle
+ self.skipTest('test not appropriate for {}'.format(sm))
+
+ from multiprocessing.forkserver import _forkserver
+ _forkserver.ensure_running()
+
+ evt = self.Event()
+ proc = self.Process(target=self._sleep_and_set_event, args=(evt, 1.0))
+ proc.start()
+
+ pid = _forkserver._forkserver_pid
+ os.kill(pid, signum)
+ time.sleep(1.0) # give it time to die
+
+ evt2 = self.Event()
+ proc2 = self.Process(target=self._sleep_and_set_event, args=(evt2,))
+ proc2.start()
+ proc2.join()
+ self.assertTrue(evt2.is_set())
+ self.assertEqual(proc2.exitcode, 0)
+
+ proc.join()
+ self.assertTrue(evt.is_set())
+ self.assertIn(proc.exitcode, (0, 255))
+
+ def test_forkserver_sigint(self):
+ # Catchable signal
+ self.check_forkserver_death(signal.SIGINT)
+
+ def test_forkserver_sigkill(self):
+ # Uncatchable signal
+ if os.name != 'nt':
+ self.check_forkserver_death(signal.SIGKILL)
+
#
#
diff --git a/Misc/NEWS.d/next/Library/2017-08-30-17-59-36.bpo-31308.KbexyC.rst b/Misc/NEWS.d/next/Library/2017-08-30-17-59-36.bpo-31308.KbexyC.rst
new file mode 100644
index 00000000000..6068b7fd32e
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2017-08-30-17-59-36.bpo-31308.KbexyC.rst
@@ -0,0 +1,2 @@
+Make multiprocessing's forkserver process immune to Ctrl-C and other user interruptions.
+If it crashes, restart it when necessary.
More information about the Python-checkins
mailing list