From 6401e5671781eb217ee1afb4603cc0d1b0367ae6 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 10 Nov 2017 12:58:55 +0200 Subject: [PATCH] [2.7] bpo-31530: Stop crashes when iterating over a file on multiple threads. (#3672) --- Lib/test/test_file2k.py | 32 ++++++++++++++++++++++ .../2017-09-20-18-28-09.bpo-31530.CdLOM7.rst | 4 +++ Objects/fileobject.c | 19 +++++++++++-- 3 files changed, 52 insertions(+), 3 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2017-09-20-18-28-09.bpo-31530.CdLOM7.rst diff --git a/Lib/test/test_file2k.py b/Lib/test/test_file2k.py index e39ef7042eae..d8966e034e08 100644 --- a/Lib/test/test_file2k.py +++ b/Lib/test/test_file2k.py @@ -652,6 +652,38 @@ def io_func(): self.f.writelines('') self._test_close_open_io(io_func) + def test_iteration_torture(self): + # bpo-31530: Crash when concurrently iterate over a file. + with open(self.filename, "wb") as fp: + for i in xrange(2**20): + fp.write(b"0"*50 + b"\n") + with open(self.filename, "rb") as f: + def iterate(): + try: + for l in f: + pass + except IOError: + pass + self._run_workers(iterate, 10) + + def test_iteration_seek(self): + # bpo-31530: Crash when concurrently seek and iterate over a file. + with open(self.filename, "wb") as fp: + for i in xrange(10000): + fp.write(b"0"*50 + b"\n") + with open(self.filename, "rb") as f: + it = iter([1] + [0]*10) # one thread reads, others seek + def iterate(): + try: + if next(it): + for l in f: + pass + else: + for i in range(100): + f.seek(i*100, 0) + except IOError: + pass + self._run_workers(iterate, 10) @unittest.skipUnless(os.name == 'posix', 'test requires a posix system.') class TestFileSignalEINTR(unittest.TestCase): diff --git a/Misc/NEWS.d/next/Core and Builtins/2017-09-20-18-28-09.bpo-31530.CdLOM7.rst b/Misc/NEWS.d/next/Core and Builtins/2017-09-20-18-28-09.bpo-31530.CdLOM7.rst new file mode 100644 index 000000000000..a6cb6c9e9ba9 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2017-09-20-18-28-09.bpo-31530.CdLOM7.rst @@ -0,0 +1,4 @@ +Fixed crashes when iterating over a file on multiple threads. +seek() and next() methods of file objects now raise an exception during +concurrent operation on the same file object. +A lock can be used to prevent the error. diff --git a/Objects/fileobject.c b/Objects/fileobject.c index 7e07a5376f88..2f63c374d1e2 100644 --- a/Objects/fileobject.c +++ b/Objects/fileobject.c @@ -430,7 +430,7 @@ close_the_file(PyFileObject *f) if (Py_REFCNT(f) > 0) { PyErr_SetString(PyExc_IOError, "close() called during concurrent " - "operation on the same file object."); + "operation on the same file object"); } else { /* This should not happen unless someone is * carelessly playing with the PyFileObject @@ -438,7 +438,7 @@ close_the_file(PyFileObject *f) * pointer. */ PyErr_SetString(PyExc_SystemError, "PyFileObject locking error in " - "destructor (refcnt <= 0 at close)."); + "destructor (refcnt <= 0 at close)"); } return NULL; } @@ -762,6 +762,12 @@ file_seek(PyFileObject *f, PyObject *args) if (f->f_fp == NULL) return err_closed(); + if (f->unlocked_count > 0) { + PyErr_SetString(PyExc_IOError, + "seek() called during concurrent " + "operation on the same file object"); + return NULL; + } drop_readahead(f); whence = 0; if (!PyArg_ParseTuple(args, "O|i:seek", &offobj, &whence)) @@ -2238,6 +2244,7 @@ readahead(PyFileObject *f, Py_ssize_t bufsize) { Py_ssize_t chunksize; + assert(f->unlocked_count == 0); if (f->f_buf != NULL) { if( (f->f_bufend - f->f_bufptr) >= 1) return 0; @@ -2279,6 +2286,12 @@ readahead_get_line_skip(PyFileObject *f, Py_ssize_t skip, Py_ssize_t bufsize) char *buf; Py_ssize_t len; + if (f->unlocked_count > 0) { + PyErr_SetString(PyExc_IOError, + "next() called during concurrent " + "operation on the same file object"); + return NULL; + } if (f->f_buf == NULL) if (readahead(f, bufsize) < 0) return NULL; @@ -2692,7 +2705,7 @@ int PyObject_AsFileDescriptor(PyObject *o) } else { PyErr_SetString(PyExc_TypeError, - "argument must be an int, or have a fileno() method."); + "argument must be an int, or have a fileno() method"); return -1; }