From: R. David Murray Date: Wed, 29 Jul 2009 15:40:30 +0000 (+0000) Subject: Merged revisions 74063 via svnmerge from X-Git-Tag: v3.1.1rc1~61 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=a44c6b325f21647a48f787215f0959de887ae08b;p=python Merged revisions 74063 via svnmerge from svn+ssh://pythondev@svn.python.org/python/branches/py3k NB: the news item for r73708 seems to have inadvertently been included in a different, unrelated merge set (r74055). I restored it here. ................ r74063 | alexandre.vassalotti | 2009-07-17 08:07:01 -0400 (Fri, 17 Jul 2009) | 17 lines Merged revisions 73694,73708,73738 via svnmerge from svn+ssh://pythondev@svn.python.org/python/trunk ........ r73694 | jesse.noller | 2009-06-29 14:24:26 -0400 (Mon, 29 Jun 2009) | 1 line Issue 5740: multiprocessing.connection.* authkey fixes ........ r73708 | jesse.noller | 2009-06-30 13:11:52 -0400 (Tue, 30 Jun 2009) | 1 line Resolves issues 5155, 5313, 5331 - bad file descriptor error with processes in processes ........ r73738 | r.david.murray | 2009-06-30 22:49:10 -0400 (Tue, 30 Jun 2009) | 2 lines Make punctuation prettier and break up run-on sentence. ........ ................ --- diff --git a/Doc/library/multiprocessing.rst b/Doc/library/multiprocessing.rst index d1b22dbd35..d3cfa0a49d 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 4a06a45d98..1a5c25fbb1 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 9d58cdc0eb..9f4e1a2435 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] # # diff --git a/Misc/ACKS b/Misc/ACKS index 0fa88ede7d..43dcb869c0 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -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 diff --git a/Misc/NEWS b/Misc/NEWS index a882c9cc66..e86753918d 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -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