Mailman 3 August 2013 - Python-checkins

cpython (merge default -> default): Merge.
by charles-francois.natali Aug. 31, 2013

Aug. 31, 2013

http://hg.python.org/cpython/rev/78f3c32a92ce changeset: 85472:78f3c32a92ce parent: 85471:d37e1d846fe3 parent: 85469:1faaec66c73d user: Charles-François Natali <cf.natali(a)gmail.com> date: Sat Aug 31 17:32:30 2013 +0200 summary: Merge. files: Doc/library/xml.etree.elementtree.rst | 34 ++++---- Include/pythonrun.h | 1 + Lib/test/test_subprocess.py | 34 +++++++++ Misc/NEWS | 4 + Python/pythonrun.c | 1 + Python/random.c | 55 +++++++++++--- 6 files changed, 99 insertions(+), 30 deletions(-) diff --git a/Doc/library/xml.etree.elementtree.rst b/Doc/library/xml.etree.elementtree.rst --- a/Doc/library/xml.etree.elementtree.rst +++ b/Doc/library/xml.etree.elementtree.rst @@ -105,7 +105,7 @@ >>> root[0][1].text '2008' -Pull API for asynchronous parsing +Pull API for non-blocking parsing ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Most parsing functions provided by this module require to read the whole @@ -121,18 +121,18 @@ incrementally with :meth:`XMLPullParser.feed` calls. To get the parsed XML elements, call :meth:`XMLPullParser.read_events`. Here's an example:: - >>> asyncparser = ET.XMLPullParser(['start', 'end']) - >>> asyncparser.feed('<mytag>sometext') - >>> list(asyncparser.read_events()) + >>> parser = ET.XMLPullParser(['start', 'end']) + >>> parser.feed('<mytag>sometext') + >>> list(parser.read_events()) [('start', <Element 'mytag' at 0x7fa66db2be58>)] - >>> asyncparser.feed(' more text</mytag>') - >>> for event, elem in asyncparser.read_events(): + >>> parser.feed(' more text</mytag>') + >>> for event, elem in parser.read_events(): ... print(event) ... print(elem.tag, 'text=', elem.text) ... end -The obvious use case is applications that operate in an asynchronous fashion +The obvious use case is applications that operate in a non-blocking fashion where the XML data is being received from a socket or read incrementally from some storage device. In such cases, blocking reads are unacceptable. @@ -427,8 +427,8 @@ Note that while :func:`iterparse` builds the tree incrementally, it issues blocking reads on *source* (or the file it names). As such, it's unsuitable - for asynchronous applications where blocking reads can't be made. For fully - asynchronous parsing, see :class:`XMLPullParser`. + for applications where blocking reads can't be made. For fully non-blocking + parsing, see :class:`XMLPullParser`. .. note:: @@ -1016,14 +1016,14 @@ .. class:: XMLPullParser(events=None) - A pull parser suitable for nonblocking (asynchronous) applications. Its - input-side API is similar to that of :class:`XMLParser`, but instead of - pushing calls to a callback target, :class:`XMLPullParser` collects an - internal list of parsing events and lets the user read from it. *events* is a - sequence of events to report back. The supported events are the strings - ``"start"``, ``"end"``, ``"start-ns"`` and ``"end-ns"`` (the "ns" events are - used to get detailed namespace information). If *events* is omitted, only - ``"end"`` events are reported. + A pull parser suitable for non-blocking applications. Its input-side API is + similar to that of :class:`XMLParser`, but instead of pushing calls to a + callback target, :class:`XMLPullParser` collects an internal list of parsing + events and lets the user read from it. *events* is a sequence of events to + report back. The supported events are the strings ``"start"``, ``"end"``, + ``"start-ns"`` and ``"end-ns"`` (the "ns" events are used to get detailed + namespace information). If *events* is omitted, only ``"end"`` events are + reported. .. method:: feed(data) diff --git a/Include/pythonrun.h b/Include/pythonrun.h --- a/Include/pythonrun.h +++ b/Include/pythonrun.h @@ -246,6 +246,7 @@ PyAPI_FUNC(void) _PyGC_Fini(void); PyAPI_FUNC(void) PySlice_Fini(void); PyAPI_FUNC(void) _PyType_Fini(void); +PyAPI_FUNC(void) _PyRandom_Fini(void); PyAPI_DATA(PyThreadState *) _Py_Finalizing; #endif diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -22,6 +22,10 @@ import resource except ImportError: resource = None +try: + import threading +except ImportError: + threading = None mswindows = (sys.platform == "win32") @@ -1020,6 +1024,36 @@ if c.exception.errno not in (errno.ENOENT, errno.EACCES): raise c.exception + @unittest.skipIf(threading is None, "threading required") + def test_double_close_on_error(self): + # Issue #18851 + fds = [] + def open_fds(): + for i in range(20): + fds.extend(os.pipe()) + time.sleep(0.001) + t = threading.Thread(target=open_fds) + t.start() + try: + with self.assertRaises(EnvironmentError): + subprocess.Popen(['nonexisting_i_hope'], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + finally: + t.join() + exc = None + for fd in fds: + # If a double close occurred, some of those fds will + # already have been closed by mistake, and os.close() + # here will raise. + try: + os.close(fd) + except OSError as e: + exc = e + if exc is not None: + raise exc + def test_issue8780(self): # Ensure that stdout is inherited from the parent # if stdout=PIPE is not used diff --git a/Misc/NEWS b/Misc/NEWS --- a/Misc/NEWS +++ b/Misc/NEWS @@ -51,6 +51,10 @@ Library ------- +- Issue #18756: os.urandom() now uses a lazily-opened persistent file + descriptor, so as to avoid using many file descriptors when run in + parallel from multiple threads. + - Issue #18418: After fork(), reinit all threads states, not only active ones. Patch by A. Jesse Jiryu Davis. diff --git a/Python/pythonrun.c b/Python/pythonrun.c --- a/Python/pythonrun.c +++ b/Python/pythonrun.c @@ -625,6 +625,7 @@ PyDict_Fini(); PySlice_Fini(); _PyGC_Fini(); + _PyRandom_Fini(); /* Cleanup Unicode implementation */ _PyUnicode_Fini(); diff --git a/Python/random.c b/Python/random.c --- a/Python/random.c +++ b/Python/random.c @@ -90,6 +90,7 @@ #if !defined(MS_WINDOWS) && !defined(__VMS) +static int urandom_fd = -1; /* Read size bytes from /dev/urandom into buffer. Call Py_FatalError() on error. */ @@ -133,18 +134,30 @@ if (size <= 0) return 0; - Py_BEGIN_ALLOW_THREADS - fd = _Py_open("/dev/urandom", O_RDONLY); - Py_END_ALLOW_THREADS - if (fd < 0) - { - if (errno == ENOENT || errno == ENXIO || - errno == ENODEV || errno == EACCES) - PyErr_SetString(PyExc_NotImplementedError, - "/dev/urandom (or equivalent) not found"); + if (urandom_fd >= 0) + fd = urandom_fd; + else { + Py_BEGIN_ALLOW_THREADS + fd = _Py_open("/dev/urandom", O_RDONLY); + Py_END_ALLOW_THREADS + if (fd < 0) + { + if (errno == ENOENT || errno == ENXIO || + errno == ENODEV || errno == EACCES) + PyErr_SetString(PyExc_NotImplementedError, + "/dev/urandom (or equivalent) not found"); + else + PyErr_SetFromErrno(PyExc_OSError); + return -1; + } + if (urandom_fd >= 0) { + /* urandom_fd was initialized by another thread while we were + not holding the GIL, keep it. */ + close(fd); + fd = urandom_fd; + } else - PyErr_SetFromErrno(PyExc_OSError); - return -1; + urandom_fd = fd; } Py_BEGIN_ALLOW_THREADS @@ -168,12 +181,20 @@ PyErr_Format(PyExc_RuntimeError, "Failed to read %zi bytes from /dev/urandom", size); - close(fd); return -1; } - close(fd); return 0; } + +static void +dev_urandom_close(void) +{ + if (urandom_fd >= 0) { + close(urandom_fd); + urandom_fd = -1; + } +} + #endif /* !defined(MS_WINDOWS) && !defined(__VMS) */ /* Fill buffer with pseudo-random bytes generated by a linear congruent @@ -271,3 +292,11 @@ #endif } } + +void +_PyRandom_Fini(void) +{ +#if !defined(MS_WINDOWS) && !defined(__VMS) + dev_urandom_close(); +#endif +} -- Repository URL: http://hg.python.org/cpython

1 0

cpython: Use INADDR_BROADCAST instead of hard-coded value (it's part of POSIX and
by charles-francois.natali Aug. 31, 2013

Aug. 31, 2013

http://hg.python.org/cpython/rev/d37e1d846fe3 changeset: 85471:d37e1d846fe3 user: Charles-François Natali <cf.natali(a)gmail.com> date: Sat Aug 31 14:48:25 2013 +0200 summary: Use INADDR_BROADCAST instead of hard-coded value (it's part of POSIX and already appears without #ifdef a couple lines above). files: Modules/socketmodule.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -4940,7 +4940,7 @@ /* special-case this address as inet_addr might return INADDR_NONE * for this */ if (strcmp(ip_addr, "255.255.255.255") == 0) { - packed_addr = 0xFFFFFFFF; + packed_addr = INADDR_BROADCAST; } else { packed_addr = inet_addr(ip_addr); -- Repository URL: http://hg.python.org/cpython

1 0

cpython: Use the recent support.HOSTv6 addition.
by charles-francois.natali Aug. 31, 2013

Aug. 31, 2013

http://hg.python.org/cpython/rev/f20201db7592 changeset: 85470:f20201db7592 parent: 85464:8f39e2f987fb user: Charles-François Natali <cf.natali(a)gmail.com> date: Sat Aug 31 14:40:49 2013 +0200 summary: Use the recent support.HOSTv6 addition. files: Lib/test/support/__init__.py | 2 +- Lib/test/test_socket.py | 10 +++------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py --- a/Lib/test/support/__init__.py +++ b/Lib/test/support/__init__.py @@ -587,7 +587,7 @@ sock = None try: sock = socket.socket(socket.AF_INET6, socket.SOCK_STREAM) - sock.bind(('::1', 0)) + sock.bind((HOSTv6, 0)) return True except OSError: pass diff --git a/Lib/test/test_socket.py b/Lib/test/test_socket.py --- a/Lib/test/test_socket.py +++ b/Lib/test/test_socket.py @@ -565,11 +565,7 @@ class Inet6TestBase(InetTestBase): """Base class for IPv6 socket tests.""" - # Don't use "localhost" here - it may not have an IPv6 address - # assigned to it by default (e.g. in /etc/hosts), and if someone - # has assigned it an IPv4-mapped address, then it's unlikely to - # work with the full IPv6 API. - host = "::1" + host = support.HOSTv6 class UDP6TestBase(Inet6TestBase): """Base class for UDP-over-IPv6 tests.""" @@ -1321,9 +1317,9 @@ @unittest.skipUnless(support.IPV6_ENABLED, 'IPv6 required for this test.') def test_flowinfo(self): self.assertRaises(OverflowError, socket.getnameinfo, - ('::1',0, 0xffffffff), 0) + (support.HOSTv6, 0, 0xffffffff), 0) with socket.socket(socket.AF_INET6, socket.SOCK_STREAM) as s: - self.assertRaises(OverflowError, s.bind, ('::1', 0, -10)) + self.assertRaises(OverflowError, s.bind, (support.HOSTv6, 0, -10)) @unittest.skipUnless(HAVE_SOCKET_CAN, 'SocketCan required for this test.') -- Repository URL: http://hg.python.org/cpython

1 0

cpython: Fix XMLPullParser documentation to say "non-blocking" instead of "asynchronous".
by eli.bendersky Aug. 31, 2013

Aug. 31, 2013

http://hg.python.org/cpython/rev/1faaec66c73d changeset: 85469:1faaec66c73d user: Eli Bendersky <eliben(a)gmail.com> date: Sat Aug 31 07:37:23 2013 -0700 summary: Fix XMLPullParser documentation to say "non-blocking" instead of "asynchronous". The latter is more ambiguous. Related to issue #17741 files: Doc/library/xml.etree.elementtree.rst | 34 +++++++------- 1 files changed, 17 insertions(+), 17 deletions(-) diff --git a/Doc/library/xml.etree.elementtree.rst b/Doc/library/xml.etree.elementtree.rst --- a/Doc/library/xml.etree.elementtree.rst +++ b/Doc/library/xml.etree.elementtree.rst @@ -105,7 +105,7 @@ >>> root[0][1].text '2008' -Pull API for asynchronous parsing +Pull API for non-blocking parsing ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Most parsing functions provided by this module require to read the whole @@ -121,18 +121,18 @@ incrementally with :meth:`XMLPullParser.feed` calls. To get the parsed XML elements, call :meth:`XMLPullParser.read_events`. Here's an example:: - >>> asyncparser = ET.XMLPullParser(['start', 'end']) - >>> asyncparser.feed('<mytag>sometext') - >>> list(asyncparser.read_events()) + >>> parser = ET.XMLPullParser(['start', 'end']) + >>> parser.feed('<mytag>sometext') + >>> list(parser.read_events()) [('start', <Element 'mytag' at 0x7fa66db2be58>)] - >>> asyncparser.feed(' more text</mytag>') - >>> for event, elem in asyncparser.read_events(): + >>> parser.feed(' more text</mytag>') + >>> for event, elem in parser.read_events(): ... print(event) ... print(elem.tag, 'text=', elem.text) ... end -The obvious use case is applications that operate in an asynchronous fashion +The obvious use case is applications that operate in a non-blocking fashion where the XML data is being received from a socket or read incrementally from some storage device. In such cases, blocking reads are unacceptable. @@ -427,8 +427,8 @@ Note that while :func:`iterparse` builds the tree incrementally, it issues blocking reads on *source* (or the file it names). As such, it's unsuitable - for asynchronous applications where blocking reads can't be made. For fully - asynchronous parsing, see :class:`XMLPullParser`. + for applications where blocking reads can't be made. For fully non-blocking + parsing, see :class:`XMLPullParser`. .. note:: @@ -1016,14 +1016,14 @@ .. class:: XMLPullParser(events=None) - A pull parser suitable for nonblocking (asynchronous) applications. Its - input-side API is similar to that of :class:`XMLParser`, but instead of - pushing calls to a callback target, :class:`XMLPullParser` collects an - internal list of parsing events and lets the user read from it. *events* is a - sequence of events to report back. The supported events are the strings - ``"start"``, ``"end"``, ``"start-ns"`` and ``"end-ns"`` (the "ns" events are - used to get detailed namespace information). If *events* is omitted, only - ``"end"`` events are reported. + A pull parser suitable for non-blocking applications. Its input-side API is + similar to that of :class:`XMLParser`, but instead of pushing calls to a + callback target, :class:`XMLPullParser` collects an internal list of parsing + events and lets the user read from it. *events* is a sequence of events to + report back. The supported events are the strings ``"start"``, ``"end"``, + ``"start-ns"`` and ``"end-ns"`` (the "ns" events are used to get detailed + namespace information). If *events* is omitted, only ``"end"`` events are + reported. .. method:: feed(data) -- Repository URL: http://hg.python.org/cpython

1 0

Daily reference leaks (acc7439b1406): sum=0
by solipsis＠pitrou.net Aug. 31, 2013

Aug. 31, 2013

results for acc7439b1406 on branch "default" -------------------------------------------- Command line was: ['./python', '-m', 'test.regrtest', '-uall', '-R', '3:3:/home/antoine/cpython/refleaks/reflogy3BU0T', '-x']

1 0

cpython: Issue #18756: os.urandom() now uses a lazily-opened persistent file descriptor,
by antoine.pitrou Aug. 30, 2013

Aug. 30, 2013

http://hg.python.org/cpython/rev/acc7439b1406 changeset: 85468:acc7439b1406 user: Antoine Pitrou <solipsis(a)pitrou.net> date: Sat Aug 31 00:26:02 2013 +0200 summary: Issue #18756: os.urandom() now uses a lazily-opened persistent file descriptor, so as to avoid using many file descriptors when run in parallel from multiple threads. files: Include/pythonrun.h | 1 + Misc/NEWS | 4 ++ Python/pythonrun.c | 1 + Python/random.c | 55 +++++++++++++++++++++++++------- 4 files changed, 48 insertions(+), 13 deletions(-) diff --git a/Include/pythonrun.h b/Include/pythonrun.h --- a/Include/pythonrun.h +++ b/Include/pythonrun.h @@ -246,6 +246,7 @@ PyAPI_FUNC(void) _PyGC_Fini(void); PyAPI_FUNC(void) PySlice_Fini(void); PyAPI_FUNC(void) _PyType_Fini(void); +PyAPI_FUNC(void) _PyRandom_Fini(void); PyAPI_DATA(PyThreadState *) _Py_Finalizing; #endif diff --git a/Misc/NEWS b/Misc/NEWS --- a/Misc/NEWS +++ b/Misc/NEWS @@ -51,6 +51,10 @@ Library ------- +- Issue #18756: os.urandom() now uses a lazily-opened persistent file + descriptor, so as to avoid using many file descriptors when run in + parallel from multiple threads. + - Issue #18418: After fork(), reinit all threads states, not only active ones. Patch by A. Jesse Jiryu Davis. diff --git a/Python/pythonrun.c b/Python/pythonrun.c --- a/Python/pythonrun.c +++ b/Python/pythonrun.c @@ -625,6 +625,7 @@ PyDict_Fini(); PySlice_Fini(); _PyGC_Fini(); + _PyRandom_Fini(); /* Cleanup Unicode implementation */ _PyUnicode_Fini(); diff --git a/Python/random.c b/Python/random.c --- a/Python/random.c +++ b/Python/random.c @@ -90,6 +90,7 @@ #if !defined(MS_WINDOWS) && !defined(__VMS) +static int urandom_fd = -1; /* Read size bytes from /dev/urandom into buffer. Call Py_FatalError() on error. */ @@ -133,18 +134,30 @@ if (size <= 0) return 0; - Py_BEGIN_ALLOW_THREADS - fd = _Py_open("/dev/urandom", O_RDONLY); - Py_END_ALLOW_THREADS - if (fd < 0) - { - if (errno == ENOENT || errno == ENXIO || - errno == ENODEV || errno == EACCES) - PyErr_SetString(PyExc_NotImplementedError, - "/dev/urandom (or equivalent) not found"); + if (urandom_fd >= 0) + fd = urandom_fd; + else { + Py_BEGIN_ALLOW_THREADS + fd = _Py_open("/dev/urandom", O_RDONLY); + Py_END_ALLOW_THREADS + if (fd < 0) + { + if (errno == ENOENT || errno == ENXIO || + errno == ENODEV || errno == EACCES) + PyErr_SetString(PyExc_NotImplementedError, + "/dev/urandom (or equivalent) not found"); + else + PyErr_SetFromErrno(PyExc_OSError); + return -1; + } + if (urandom_fd >= 0) { + /* urandom_fd was initialized by another thread while we were + not holding the GIL, keep it. */ + close(fd); + fd = urandom_fd; + } else - PyErr_SetFromErrno(PyExc_OSError); - return -1; + urandom_fd = fd; } Py_BEGIN_ALLOW_THREADS @@ -168,12 +181,20 @@ PyErr_Format(PyExc_RuntimeError, "Failed to read %zi bytes from /dev/urandom", size); - close(fd); return -1; } - close(fd); return 0; } + +static void +dev_urandom_close(void) +{ + if (urandom_fd >= 0) { + close(urandom_fd); + urandom_fd = -1; + } +} + #endif /* !defined(MS_WINDOWS) && !defined(__VMS) */ /* Fill buffer with pseudo-random bytes generated by a linear congruent @@ -271,3 +292,11 @@ #endif } } + +void +_PyRandom_Fini(void) +{ +#if !defined(MS_WINDOWS) && !defined(__VMS) + dev_urandom_close(); +#endif +} -- Repository URL: http://hg.python.org/cpython

1 0

cpython (merge 3.3 -> default): Forward port new tests from Issue #18851.
by antoine.pitrou Aug. 30, 2013

Aug. 30, 2013

http://hg.python.org/cpython/rev/290ca31d4cfe changeset: 85467:290ca31d4cfe parent: 85464:8f39e2f987fb parent: 85466:c11754defe1c user: Antoine Pitrou <solipsis(a)pitrou.net> date: Sat Aug 31 00:12:37 2013 +0200 summary: Forward port new tests from Issue #18851. files: Lib/test/test_subprocess.py | 34 +++++++++++++++++++++++++ 1 files changed, 34 insertions(+), 0 deletions(-) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -22,6 +22,10 @@ import resource except ImportError: resource = None +try: + import threading +except ImportError: + threading = None mswindows = (sys.platform == "win32") @@ -1020,6 +1024,36 @@ if c.exception.errno not in (errno.ENOENT, errno.EACCES): raise c.exception + @unittest.skipIf(threading is None, "threading required") + def test_double_close_on_error(self): + # Issue #18851 + fds = [] + def open_fds(): + for i in range(20): + fds.extend(os.pipe()) + time.sleep(0.001) + t = threading.Thread(target=open_fds) + t.start() + try: + with self.assertRaises(EnvironmentError): + subprocess.Popen(['nonexisting_i_hope'], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + finally: + t.join() + exc = None + for fd in fds: + # If a double close occurred, some of those fds will + # already have been closed by mistake, and os.close() + # here will raise. + try: + os.close(fd) + except OSError as e: + exc = e + if exc is not None: + raise exc + def test_issue8780(self): # Ensure that stdout is inherited from the parent # if stdout=PIPE is not used -- Repository URL: http://hg.python.org/cpython

1 0

cpython (3.3): Forward port new tests from Issue #18851.
by antoine.pitrou Aug. 30, 2013

Aug. 30, 2013

http://hg.python.org/cpython/rev/c11754defe1c changeset: 85466:c11754defe1c branch: 3.3 parent: 85463:29fce7f31539 user: Antoine Pitrou <solipsis(a)pitrou.net> date: Fri Aug 30 23:38:13 2013 +0200 summary: Forward port new tests from Issue #18851. files: Lib/test/test_subprocess.py | 34 +++++++++++++++++++++++++ 1 files changed, 34 insertions(+), 0 deletions(-) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -22,6 +22,10 @@ import resource except ImportError: resource = None +try: + import threading +except ImportError: + threading = None mswindows = (sys.platform == "win32") @@ -987,6 +991,36 @@ if c.exception.errno not in (errno.ENOENT, errno.EACCES): raise c.exception + @unittest.skipIf(threading is None, "threading required") + def test_double_close_on_error(self): + # Issue #18851 + fds = [] + def open_fds(): + for i in range(20): + fds.extend(os.pipe()) + time.sleep(0.001) + t = threading.Thread(target=open_fds) + t.start() + try: + with self.assertRaises(EnvironmentError): + subprocess.Popen(['nonexisting_i_hope'], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + finally: + t.join() + exc = None + for fd in fds: + # If a double close occurred, some of those fds will + # already have been closed by mistake, and os.close() + # here will raise. + try: + os.close(fd) + except OSError as e: + exc = e + if exc is not None: + raise exc + def test_issue8780(self): # Ensure that stdout is inherited from the parent # if stdout=PIPE is not used -- Repository URL: http://hg.python.org/cpython

1 0

cpython (2.7): Issue #18851: Avoid a double close of subprocess pipes when the child process
by antoine.pitrou Aug. 30, 2013

Aug. 30, 2013

http://hg.python.org/cpython/rev/43749cb6bdbd changeset: 85465:43749cb6bdbd branch: 2.7 parent: 85462:ba54011aa295 user: Antoine Pitrou <solipsis(a)pitrou.net> date: Fri Aug 30 23:38:13 2013 +0200 summary: Issue #18851: Avoid a double close of subprocess pipes when the child process fails starting. files: Lib/subprocess.py | 73 +++++++++++++++--------- Lib/test/test_subprocess.py | 38 ++++++++++++- Misc/NEWS | 3 + 3 files changed, 85 insertions(+), 29 deletions(-) diff --git a/Lib/subprocess.py b/Lib/subprocess.py --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -698,12 +698,12 @@ (p2cread, p2cwrite, c2pread, c2pwrite, - errread, errwrite) = self._get_handles(stdin, stdout, stderr) + errread, errwrite), to_close = self._get_handles(stdin, stdout, stderr) try: self._execute_child(args, executable, preexec_fn, close_fds, cwd, env, universal_newlines, - startupinfo, creationflags, shell, + startupinfo, creationflags, shell, to_close, p2cread, p2cwrite, c2pread, c2pwrite, errread, errwrite) @@ -711,18 +711,12 @@ # Preserve original exception in case os.close raises. exc_type, exc_value, exc_trace = sys.exc_info() - to_close = [] - # Only close the pipes we created. - if stdin == PIPE: - to_close.extend((p2cread, p2cwrite)) - if stdout == PIPE: - to_close.extend((c2pread, c2pwrite)) - if stderr == PIPE: - to_close.extend((errread, errwrite)) - for fd in to_close: try: - os.close(fd) + if mswindows: + fd.Close() + else: + os.close(fd) except EnvironmentError: pass @@ -816,8 +810,9 @@ """Construct and return tuple with IO objects: p2cread, p2cwrite, c2pread, c2pwrite, errread, errwrite """ + to_close = set() if stdin is None and stdout is None and stderr is None: - return (None, None, None, None, None, None) + return (None, None, None, None, None, None), to_close p2cread, p2cwrite = None, None c2pread, c2pwrite = None, None @@ -835,6 +830,10 @@ # Assuming file-like object p2cread = msvcrt.get_osfhandle(stdin.fileno()) p2cread = self._make_inheritable(p2cread) + # We just duplicated the handle, it has to be closed at the end + to_close.add(p2cread) + if stdin == PIPE: + to_close.add(p2cwrite) if stdout is None: c2pwrite = _subprocess.GetStdHandle(_subprocess.STD_OUTPUT_HANDLE) @@ -848,6 +847,10 @@ # Assuming file-like object c2pwrite = msvcrt.get_osfhandle(stdout.fileno()) c2pwrite = self._make_inheritable(c2pwrite) + # We just duplicated the handle, it has to be closed at the end + to_close.add(c2pwrite) + if stdout == PIPE: + to_close.add(c2pread) if stderr is None: errwrite = _subprocess.GetStdHandle(_subprocess.STD_ERROR_HANDLE) @@ -863,10 +866,14 @@ # Assuming file-like object errwrite = msvcrt.get_osfhandle(stderr.fileno()) errwrite = self._make_inheritable(errwrite) + # We just duplicated the handle, it has to be closed at the end + to_close.add(errwrite) + if stderr == PIPE: + to_close.add(errread) return (p2cread, p2cwrite, c2pread, c2pwrite, - errread, errwrite) + errread, errwrite), to_close def _make_inheritable(self, handle): @@ -895,7 +902,7 @@ def _execute_child(self, args, executable, preexec_fn, close_fds, cwd, env, universal_newlines, - startupinfo, creationflags, shell, + startupinfo, creationflags, shell, to_close, p2cread, p2cwrite, c2pread, c2pwrite, errread, errwrite): @@ -934,6 +941,10 @@ # kill children. creationflags |= _subprocess.CREATE_NEW_CONSOLE + def _close_in_parent(fd): + fd.Close() + to_close.remove(fd) + # Start the process try: hp, ht, pid, tid = _subprocess.CreateProcess(executable, args, @@ -958,11 +969,11 @@ # pipe will not close when the child process exits and the # ReadFile will hang. if p2cread is not None: - p2cread.Close() + _close_in_parent(p2cread) if c2pwrite is not None: - c2pwrite.Close() + _close_in_parent(c2pwrite) if errwrite is not None: - errwrite.Close() + _close_in_parent(errwrite) # Retain the process handle, but close the thread handle self._child_created = True @@ -1088,6 +1099,7 @@ """Construct and return tuple with IO objects: p2cread, p2cwrite, c2pread, c2pwrite, errread, errwrite """ + to_close = set() p2cread, p2cwrite = None, None c2pread, c2pwrite = None, None errread, errwrite = None, None @@ -1096,6 +1108,7 @@ pass elif stdin == PIPE: p2cread, p2cwrite = self.pipe_cloexec() + to_close.update((p2cread, p2cwrite)) elif isinstance(stdin, int): p2cread = stdin else: @@ -1106,6 +1119,7 @@ pass elif stdout == PIPE: c2pread, c2pwrite = self.pipe_cloexec() + to_close.update((c2pread, c2pwrite)) elif isinstance(stdout, int): c2pwrite = stdout else: @@ -1116,6 +1130,7 @@ pass elif stderr == PIPE: errread, errwrite = self.pipe_cloexec() + to_close.update((errread, errwrite)) elif stderr == STDOUT: errwrite = c2pwrite elif isinstance(stderr, int): @@ -1126,7 +1141,7 @@ return (p2cread, p2cwrite, c2pread, c2pwrite, - errread, errwrite) + errread, errwrite), to_close def _set_cloexec_flag(self, fd, cloexec=True): @@ -1170,7 +1185,7 @@ def _execute_child(self, args, executable, preexec_fn, close_fds, cwd, env, universal_newlines, - startupinfo, creationflags, shell, + startupinfo, creationflags, shell, to_close, p2cread, p2cwrite, c2pread, c2pwrite, errread, errwrite): @@ -1189,6 +1204,10 @@ if executable is None: executable = args[0] + def _close_in_parent(fd): + os.close(fd) + to_close.remove(fd) + # For transferring possible exec failure from child to parent # The first char specifies the exception type: 0 means # OSError, 1 means some other error. @@ -1283,17 +1302,17 @@ # be sure the FD is closed no matter what os.close(errpipe_write) - if p2cread is not None and p2cwrite is not None: - os.close(p2cread) - if c2pwrite is not None and c2pread is not None: - os.close(c2pwrite) - if errwrite is not None and errread is not None: - os.close(errwrite) - # Wait for exec to fail or succeed; possibly raising exception # Exception limited to 1M data = _eintr_retry_call(os.read, errpipe_read, 1048576) finally: + if p2cread is not None and p2cwrite is not None: + _close_in_parent(p2cread) + if c2pwrite is not None and c2pread is not None: + _close_in_parent(c2pwrite) + if errwrite is not None and errread is not None: + _close_in_parent(errwrite) + # be sure the FD is closed no matter what os.close(errpipe_read) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -14,6 +14,10 @@ import resource except ImportError: resource = None +try: + import threading +except ImportError: + threading = None mswindows = (sys.platform == "win32") @@ -629,6 +633,36 @@ if c.exception.errno not in (errno.ENOENT, errno.EACCES): raise c.exception + @unittest.skipIf(threading is None, "threading required") + def test_double_close_on_error(self): + # Issue #18851 + fds = [] + def open_fds(): + for i in range(20): + fds.extend(os.pipe()) + time.sleep(0.001) + t = threading.Thread(target=open_fds) + t.start() + try: + with self.assertRaises(EnvironmentError): + subprocess.Popen(['nonexisting_i_hope'], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + finally: + t.join() + exc = None + for fd in fds: + # If a double close occurred, some of those fds will + # already have been closed by mistake, and os.close() + # here will raise. + try: + os.close(fd) + except OSError as e: + exc = e + if exc is not None: + raise exc + def test_handles_closed_on_exception(self): # If CreateProcess exits with an error, ensure the # duplicate output handles are released @@ -783,7 +817,7 @@ def _execute_child( self, args, executable, preexec_fn, close_fds, cwd, env, - universal_newlines, startupinfo, creationflags, shell, + universal_newlines, startupinfo, creationflags, shell, to_close, p2cread, p2cwrite, c2pread, c2pwrite, errread, errwrite): @@ -791,7 +825,7 @@ subprocess.Popen._execute_child( self, args, executable, preexec_fn, close_fds, cwd, env, universal_newlines, - startupinfo, creationflags, shell, + startupinfo, creationflags, shell, to_close, p2cread, p2cwrite, c2pread, c2pwrite, errread, errwrite) diff --git a/Misc/NEWS b/Misc/NEWS --- a/Misc/NEWS +++ b/Misc/NEWS @@ -32,6 +32,9 @@ Library ------- +- Issue #18851: Avoid a double close of subprocess pipes when the child + process fails starting. + - Issue #18418: After fork(), reinit all threads states, not only active ones. Patch by A. Jesse Jiryu Davis. -- Repository URL: http://hg.python.org/cpython

1 0

cpython (merge 3.3 -> default): Issue #18418: After fork(), reinit all threads states, not only active ones.
by charles-francois.natali Aug. 30, 2013

Aug. 30, 2013

http://hg.python.org/cpython/rev/8f39e2f987fb changeset: 85464:8f39e2f987fb parent: 85461:8fd72b1bb262 parent: 85463:29fce7f31539 user: Charles-François Natali <cf.natali(a)gmail.com> date: Fri Aug 30 23:34:26 2013 +0200 summary: Issue #18418: After fork(), reinit all threads states, not only active ones. Patch by A. Jesse Jiryu Davis. files: Lib/test/test_threading.py | 21 +++++++++++++++++++++ Lib/threading.py | 2 +- Misc/ACKS | 1 + Misc/NEWS | 3 +++ 4 files changed, 26 insertions(+), 1 deletions(-) diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py --- a/Lib/test/test_threading.py +++ b/Lib/test/test_threading.py @@ -445,6 +445,27 @@ self.assertEqual(out, b'') self.assertEqual(err, b'') + @unittest.skipUnless(hasattr(os, 'fork'), "needs os.fork()") + def test_is_alive_after_fork(self): + # Try hard to trigger #18418: is_alive() could sometimes be True on + # threads that vanished after a fork. + old_interval = sys.getswitchinterval() + self.addCleanup(sys.setswitchinterval, old_interval) + + # Make the bug more likely to manifest. + sys.setswitchinterval(1e-6) + + for i in range(20): + t = threading.Thread(target=lambda: None) + t.start() + self.addCleanup(t.join) + pid = os.fork() + if pid == 0: + os._exit(1 if t.is_alive() else 0) + else: + pid, status = os.waitpid(pid, 0) + self.assertEqual(0, status) + class ThreadJoinOnShutdown(BaseTestCase): diff --git a/Lib/threading.py b/Lib/threading.py --- a/Lib/threading.py +++ b/Lib/threading.py @@ -940,7 +940,7 @@ new_active = {} current = current_thread() with _active_limbo_lock: - for thread in _active.values(): + for thread in _enumerate(): # Any lock/condition variable may be currently locked or in an # invalid state, so we reinitialize them. thread._reset_internal_locks() diff --git a/Misc/ACKS b/Misc/ACKS --- a/Misc/ACKS +++ b/Misc/ACKS @@ -288,6 +288,7 @@ Kushal Das Jonathan Dasteel Pierre-Yves David +A. Jesse Jiryu Davis John DeGood Ned Deily Vincent Delft diff --git a/Misc/NEWS b/Misc/NEWS --- a/Misc/NEWS +++ b/Misc/NEWS @@ -51,6 +51,9 @@ Library ------- +- Issue #18418: After fork(), reinit all threads states, not only active ones. + Patch by A. Jesse Jiryu Davis. + - Issue #17974: Switch unittest from using getopt to using argparse. - Issue #11798: TestSuite now drops references to own tests after execution. -- Repository URL: http://hg.python.org/cpython

1 0