From a8392717f1ca5263fb0e942e7e64f9fdc8d115ef Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Fri, 30 Aug 2013 23:38:13 +0200 Subject: [PATCH 1/3] Forward port new tests from Issue #18851. --- Lib/test/test_subprocess.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 723845a13f3..4c9f29bdb8d 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -22,6 +22,10 @@ try: import resource except ImportError: resource = None +try: + import threading +except ImportError: + threading = None mswindows = (sys.platform == "win32") @@ -987,6 +991,36 @@ class ProcessTestCase(BaseTestCase): 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 From 4879a963d4a2020f4c26f2583f3ac35ec8d6edfe Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Sat, 31 Aug 2013 00:26:02 +0200 Subject: [PATCH 2/3] 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. --- 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 index aca591507e3..8fdb5b52108 100644 --- a/Include/pythonrun.h +++ b/Include/pythonrun.h @@ -246,6 +246,7 @@ PyAPI_FUNC(void) _PyGC_DumpShutdownStats(void); 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 index dfba2cac0c9..7258ce1755f 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -51,6 +51,10 @@ Core and Builtins 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 index 375bf34c82d..cbd62aa056f 100644 --- a/Python/pythonrun.c +++ b/Python/pythonrun.c @@ -625,6 +625,7 @@ Py_Finalize(void) PyDict_Fini(); PySlice_Fini(); _PyGC_Fini(); + _PyRandom_Fini(); /* Cleanup Unicode implementation */ _PyUnicode_Fini(); diff --git a/Python/random.c b/Python/random.c index 8cf2fef7c6a..1d470c703cc 100644 --- a/Python/random.c +++ b/Python/random.c @@ -90,6 +90,7 @@ vms_urandom(unsigned char *buffer, Py_ssize_t size, int raise) #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 @@ dev_urandom_python(char *buffer, Py_ssize_t size) 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 @@ dev_urandom_python(char *buffer, Py_ssize_t size) 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 @@ _PyRandom_Init(void) #endif } } + +void +_PyRandom_Fini(void) +{ +#if !defined(MS_WINDOWS) && !defined(__VMS) + dev_urandom_close(); +#endif +} From 2c68e300e54438d0c91ffa9cd5c78085d5596236 Mon Sep 17 00:00:00 2001 From: Eli Bendersky Date: Sat, 31 Aug 2013 07:37:23 -0700 Subject: [PATCH 3/3] Fix XMLPullParser documentation to say "non-blocking" instead of "asynchronous". The latter is more ambiguous. Related to issue #17741 --- Doc/library/xml.etree.elementtree.rst | 34 +++++++++++++-------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/Doc/library/xml.etree.elementtree.rst b/Doc/library/xml.etree.elementtree.rst index 9936d03cdff..97550edaaa5 100644 --- a/Doc/library/xml.etree.elementtree.rst +++ b/Doc/library/xml.etree.elementtree.rst @@ -105,7 +105,7 @@ Children are nested, and we can access specific child nodes by index:: >>> 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 @@ require a blocking read to obtain the XML data, and is instead fed with data 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('sometext') - >>> list(asyncparser.read_events()) + >>> parser = ET.XMLPullParser(['start', 'end']) + >>> parser.feed('sometext') + >>> list(parser.read_events()) [('start', )] - >>> asyncparser.feed(' more text') - >>> for event, elem in asyncparser.read_events(): + >>> parser.feed(' more text') + >>> 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 @@ Functions 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 @@ XMLPullParser Objects .. 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)