python · vstinner · Jun 12, 2019 · Jun 12, 2019
diff --git a/Lib/_pyio.py b/Lib/_pyio.py
@@ -1239,12 +1239,15 @@ def __init__(self, raw, buffer_size=DEFAULT_BUFFER_SIZE):
     def writable(self):
         return self.raw.writable()
 
+    def _check_closed(self, err_msg):
+        if self.closed or self._write_buf is None:
+            raise ValueError(err_msg)
+
     def write(self, b):
         if isinstance(b, str):
             raise TypeError("can't write str to binary stream")
         with self._write_lock:
-            if self.closed:
-                raise ValueError("write to closed file")
+            self._check_closed("write to closed file")
             # XXX we can implement some more tricks to try and avoid
             # partial writes
             if len(self._write_buf) > self.buffer_size:
@@ -1279,8 +1282,7 @@ def flush(self):
             self._flush_unlocked()
 
     def _flush_unlocked(self):
-        if self.closed:
-            raise ValueError("flush on closed file")
+        self._check_closed("flush on closed file")
         while self._write_buf:
             try:
                 n = self.raw.write(self._write_buf)
@@ -1296,7 +1298,10 @@ def _flush_unlocked(self):
             del self._write_buf[:n]
 
     def tell(self):
-        return _BufferedIOMixin.tell(self) + len(self._write_buf)
+        pos = _BufferedIOMixin.tell(self)
+        if self._write_buf is not None:
+            pos += len(self._write_buf)
+        return pos
 
     def seek(self, pos, whence=0):
         if whence not in valid_seek_flags:
@@ -1309,15 +1314,22 @@ def close(self):
         with self._write_lock:
             if self.raw is None or self.closed:
                 return
+            has_write_buf = (self._write_buf is not None)
+
         # We have to release the lock and call self.flush() (which will
         # probably just re-take the lock) in case flush has been overridden in
         # a subclass or the user set self.flush to something. This is the same
         # behavior as the C implementation.
         try:
-            # may raise BlockingIOError or BrokenPipeError etc
-            self.flush()
+            # bpo-37223: If close() has already been called, _write_buf is set
+            # to None. In this case, flush() would raise
+            # ValueError("flush on closed file").
+            if has_write_buf:
+                # may raise BlockingIOError or BrokenPipeError etc
+                self.flush()
         finally:
             with self._write_lock:
+                self._write_buf = None
                 self.raw.close()
 
 

diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py
@@ -1196,7 +1196,7 @@ def test_multi_close(self):
         b.close()
         b.close()
         b.close()
-        self.assertRaises(ValueError, b.flush)
+        self.assertRaisesRegex(ValueError, 'closed', b.flush)
 
     def test_unseekable(self):
         bufio = self.tp(self.MockUnseekableIO(b"A" * 10))
@@ -1869,6 +1869,39 @@ def test_slow_close_from_thread(self):
         self.assertTrue(bufio.closed)
         t.join()
 
+    def test_close_dont_call_flush(self):
+        # bpo-37223: This test is specific to the C implementation (_io).
+        raw = self.MockRawIO()
+        def bad_flush():
+            raise OSError()
+        orig_flush = raw.flush
+        raw.flush = bad_flush
+        def bad_close():
+            raw.flush()
+        orig_close = raw.close
+        raw.close = bad_close
+
+        b = self.tp(raw)
+        b.write(b'spam')
+        self.assertRaises(OSError, b.close) # exception not swallowed
+        self.assertFalse(b.closed)
+        self.assertFalse(raw.closed)
+
+        def bad_flush2():
+            raise AssertionError
+
+        b.flush = bad_flush2
+        raw.close = orig_close
+        raw.flush = orig_flush
+
+        # if b is already closed, b.flush() must not be called
+        b.close()
+
+    def test_closed_write(self):
+        raw = self.MockRawIO()
+        b = self.tp(raw)
+        b.close()
+        self.assertRaisesRegex(ValueError, 'closed', b.write, b'abc')
 
 
 class CBufferedWriterTest(BufferedWriterTest, SizeofTest):

diff --git a/Misc/NEWS.d/next/Library/2019-06-12-04-33-59.bpo-37223.oZqbr9.rst b/Misc/NEWS.d/next/Library/2019-06-12-04-33-59.bpo-37223.oZqbr9.rst
@@ -0,0 +1,3 @@
+:class:`io.BufferedWrite.close` no longer call its
+:meth:`~io.BufferedWrite.flush` method if :class:`~io.BufferedWrite.close` has
+already been closed.
diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c
@@ -511,15 +511,26 @@ buffered_close(buffered *self, PyObject *args)
         else
             PyErr_Clear();
     }
-    /* flush() will most probably re-take the lock, so drop it first */
-    LEAVE_BUFFERED(self)
-    res = PyObject_CallMethodObjArgs((PyObject *)self, _PyIO_str_flush, NULL);
-    if (!ENTER_BUFFERED(self))
-        return NULL;
-    if (res == NULL)
-        PyErr_Fetch(&exc, &val, &tb);
-    else
-        Py_DECREF(res);
+
+    /* bpo-37223: If buffered_close() has already been called, self->buffer is set
+       to NULL. In this case, calling buffered_flush() would raise
+       ValueError("flush of closed file"). */
+    if (self->buffer != NULL) {
+        /* flush() will most probably re-take the lock, so drop it first */
+        LEAVE_BUFFERED(self)
+
+        res = PyObject_CallMethodObjArgs((PyObject *)self, _PyIO_str_flush, NULL);
+
+        if (!ENTER_BUFFERED(self)) {
+            return NULL;
+        }
+        if (res == NULL) {
+            PyErr_Fetch(&exc, &val, &tb);
+        }
+        else {
+            Py_DECREF(res);
+        }
+    }
 
     res = PyObject_CallMethodObjArgs(self->raw, _PyIO_str_close, NULL);