From dbf52e02f18dac6f5f0a64f78932f3dc6efc056b Mon Sep 17 00:00:00 2001
From: Benjamin Peterson <benjamin@python.org>
Date: Tue, 2 Jan 2018 09:25:41 -0800
Subject: [PATCH] bpo-31530: fix crash when multiple threads iterate over a
 file, round 2 (#5060)

Multiple threads iterating over a file can corrupt the file's internal readahead
buffer resulting in crashes. To fix this, cache buffer state thread-locally for
the duration of a file_iternext call and only update the file's internal state
after reading completes.

No attempt is made to define or provide "reasonable" semantics for iterating
over a file on multiple threads. (Non-crashing) races are still
present. Duplicated, corrupt, and missing data will happen.

This was originally fixed by 6401e5671781eb217ee1afb4603cc0d1b0367ae6, which
raised an exception from seek() and next() when concurrent operations were
detected. Alas, this simpler solution breaks legitimate use cases such as
capturing the standard streams when multiple threads are logging.
---
 Lib/test/test_file2k.py                            |  27 ++---
 .../2017-09-20-18-28-09.bpo-31530.CdLOM7.rst       |   3 -
 Objects/fileobject.c                               | 118 ++++++++++++---------
 3 files changed, 78 insertions(+), 70 deletions(-)

--- a/Lib/test/test_file2k.py
+++ b/Lib/test/test_file2k.py
@@ -680,18 +680,15 @@ class FileThreadingTests(unittest.TestCa
             self._run_workers(iterate, 10)
 
     def test_iteration_torture(self):
-        # bpo-31530: Crash when concurrently iterate over a file.
+        # bpo-31530
         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:
+            def it():
+                for l in f:
                     pass
-            self._run_workers(iterate, 10)
+            self._run_workers(it, 10)
 
     def test_iteration_seek(self):
         # bpo-31530: Crash when concurrently seek and iterate over a file.
@@ -701,17 +698,15 @@ class FileThreadingTests(unittest.TestCa
         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
+                if next(it):
+                    for l in f:
+                        pass
+                else:
+                    for i in xrange(100):
+                        f.seek(i*100, 0)
             self._run_workers(iterate, 10)
 
+
 @unittest.skipUnless(os.name == 'posix', 'test requires a posix system.')
 class TestFileSignalEINTR(unittest.TestCase):
     def _test_reading(self, data_to_write, read_and_verify_code, method_name,
--- a/Objects/fileobject.c
+++ b/Objects/fileobject.c
@@ -772,12 +772,6 @@ file_seek(PyFileObject *f, PyObject *arg
 
     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_file_readahead(f);
     whence = 0;
     if (!PyArg_ParseTuple(args, "O|i:seek", &offobj, &whence))
@@ -2269,7 +2263,6 @@ readahead(PyFileObject *f, readaheadbuff
 {
     Py_ssize_t chunksize;
 
-    assert(f->unlocked_count == 0);
     if (rab->buf != NULL) {
         if ((rab->bufend - rab->bufptr) >= 1)
             return 0;
@@ -2310,12 +2303,6 @@ readahead_get_line_skip(PyFileObject *f,
     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 (rab->buf == NULL)
         if (readahead(f, rab, bufsize) < 0)
             return NULL;
