diff options
-rw-r--r-- | Doc/library/multiprocessing.rst | 36 | ||||
-rw-r--r-- | Lib/multiprocessing/process.py | 3 | ||||
-rw-r--r-- | Lib/test/test_multiprocessing.py | 70 | ||||
-rw-r--r-- | Misc/ACKS | 3 | ||||
-rw-r--r-- | Misc/NEWS | 4 |
5 files changed, 112 insertions, 4 deletions
diff --git a/Doc/library/multiprocessing.rst b/Doc/library/multiprocessing.rst index d1b22db..d3cfa0a 100644 --- a/Doc/library/multiprocessing.rst +++ b/Doc/library/multiprocessing.rst @@ -1713,7 +1713,7 @@ authentication* using the :mod:`hmac` module. generally be omitted since it can usually be inferred from the format of *address*. (See :ref:`multiprocessing-address-formats`) - If *authentication* is ``True`` or *authkey* is a string then digest + If *authenticate* is ``True`` or *authkey* is a string then digest authentication is used. The key used for authentication will be either *authkey* or ``current_process().authkey)`` if *authkey* is ``None``. If authentication fails then :exc:`AuthenticationError` is raised. See @@ -1755,7 +1755,7 @@ authentication* using the :mod:`hmac` module. If *authkey* is ``None`` and *authenticate* is ``True`` then ``current_process().authkey`` is used as the authentication key. If - *authkey* is ``None`` and *authentication* is ``False`` then no + *authkey* is ``None`` and *authenticate* is ``False`` then no authentication is done. If authentication fails then :exc:`AuthenticationError` is raised. See :ref:`multiprocessing-auth-keys`. @@ -2097,6 +2097,38 @@ Explicitly pass resources to child processes for i in range(10): Process(target=f, args=(lock,)).start() +Beware replacing sys.stdin with a "file like object" + + :mod:`multiprocessing` originally unconditionally called:: + + os.close(sys.stdin.fileno()) + + in the :meth:`multiprocessing.Process._bootstrap` method --- this resulted + in issues with processes-in-processes. This has been changed to:: + + sys.stdin.close() + sys.stdin = open(os.devnull) + + Which solves the fundamental issue of processes colliding with each other + resulting in a bad file descriptor error, but introduces a potential danger + to applications which replace :func:`sys.stdin` with a "file-like object" + with output buffering. This danger is that if multiple processes call + :func:`close()` on this file-like object, it could result in the same + data being flushed to the object multiple times, resulting in corruption. + + If you write a file-like object and implement your own caching, you can + make it fork-safe by storing the pid whenever you append to the cache, + and discarding the cache when the pid changes. For example:: + + @property + def cache(self): + pid = os.getpid() + if pid != self._pid: + self._pid = pid + self._cache = [] + return self._cache + + For more information, see :issue:`5155`, :issue:`5313` and :issue:`5331` Windows ~~~~~~~ diff --git a/Lib/multiprocessing/process.py b/Lib/multiprocessing/process.py index 4a06a45..1a5c25f 100644 --- a/Lib/multiprocessing/process.py +++ b/Lib/multiprocessing/process.py @@ -221,7 +221,8 @@ class Process(object): self._counter = itertools.count(1) if sys.stdin is not None: try: - os.close(sys.stdin.fileno()) + sys.stdin.close() + sys.stdin = open(os.devnull) except (OSError, ValueError): pass _current_process = self diff --git a/Lib/test/test_multiprocessing.py b/Lib/test/test_multiprocessing.py index 9d58cdc..9f4e1a2 100644 --- a/Lib/test/test_multiprocessing.py +++ b/Lib/test/test_multiprocessing.py @@ -8,6 +8,7 @@ import unittest import threading import queue as pyqueue import time +import io import sys import os import gc @@ -1862,7 +1863,74 @@ class TestInitializers(unittest.TestCase): p.join() self.assertEqual(self.ns.test, 1) -testcases_other = [OtherTest, TestInvalidHandle, TestInitializers] +# +# Issue 5155, 5313, 5331: Test process in processes +# Verifies os.close(sys.stdin.fileno) vs. sys.stdin.close() behavior +# + +def _ThisSubProcess(q): + try: + item = q.get(block=False) + except pyqueue.Empty: + pass + +def _TestProcess(q): + queue = multiprocessing.Queue() + subProc = multiprocessing.Process(target=_ThisSubProcess, args=(queue,)) + subProc.start() + subProc.join() + +def _afunc(x): + return x*x + +def pool_in_process(): + pool = multiprocessing.Pool(processes=4) + x = pool.map(_afunc, [1, 2, 3, 4, 5, 6, 7]) + +class _file_like(object): + def __init__(self, delegate): + self._delegate = delegate + self._pid = None + + @property + def cache(self): + pid = os.getpid() + # There are no race conditions since fork keeps only the running thread + if pid != self._pid: + self._pid = pid + self._cache = [] + return self._cache + + def write(self, data): + self.cache.append(data) + + def flush(self): + self._delegate.write(''.join(self.cache)) + self._cache = [] + +class TestStdinBadfiledescriptor(unittest.TestCase): + + def test_queue_in_process(self): + queue = multiprocessing.Queue() + proc = multiprocessing.Process(target=_TestProcess, args=(queue,)) + proc.start() + proc.join() + + def test_pool_in_process(self): + p = multiprocessing.Process(target=pool_in_process) + p.start() + p.join() + + def test_flushing(self): + sio = io.StringIO() + flike = _file_like(sio) + flike.write('foo') + proc = multiprocessing.Process(target=lambda: flike.flush()) + flike.flush() + assert sio.getvalue() == 'foo' + +testcases_other = [OtherTest, TestInvalidHandle, TestInitializers, + TestStdinBadfiledescriptor] # # @@ -45,6 +45,7 @@ Des Barry Ulf Bartelt Nick Bastin Jeff Bauer +Mike Bayer Michael R Bax Anthony Baxter Samuel L. Bayer @@ -184,6 +185,7 @@ Cesar Douady Dean Draayer John DuBois Paul Dubois +Graham Dumpleton Quinn Dunkan Robin Dunn Luke Dunstan @@ -556,6 +558,7 @@ Steven Pemberton Santiago Peresón Mark Perrego Trevor Perrin +Gabriel de Perthuis Tim Peters Benjamin Peterson Chris Petrilli @@ -42,6 +42,10 @@ C-API Library ------- +- Issues #5155, #5313, #5331: multiprocessing.Process._bootstrap was + unconditionally calling "os.close(sys.stdin.fileno())" resulting in file + descriptor errors + - Issue #1424152: Fix for http.client, urllib.request to support SSL while working through proxy. Original patch by Christopher Li, changes made by Senthil Kumaran |