]> granicus.if.org Git - python/commitdiff
call close on the underlying stream even if flush raises (closes #16597)
authorBenjamin Peterson <benjamin@python.org>
Thu, 20 Dec 2012 17:53:11 +0000 (11:53 -0600)
committerBenjamin Peterson <benjamin@python.org>
Thu, 20 Dec 2012 17:53:11 +0000 (11:53 -0600)
Patch by Serhiy Storchaka.

Lib/_pyio.py
Lib/test/test_io.py
Misc/NEWS
Modules/_io/bufferedio.c
Modules/_io/textio.c

index fa77ec17f5f9a09ed09f69e9298de9142c5283b9..9cbb364dd1b74d412f64057dd4953d613912b205 100644 (file)
@@ -346,8 +346,10 @@ class IOBase(metaclass=abc.ABCMeta):
         This method has no effect if the file is already closed.
         """
         if not self.__closed:
-            self.flush()
-            self.__closed = True
+            try:
+                self.flush()
+            finally:
+                self.__closed = True
 
     def __del__(self):
         """Destructor.  Calls close()."""
@@ -1584,8 +1586,10 @@ class TextIOWrapper(TextIOBase):
 
     def close(self):
         if self.buffer is not None and not self.closed:
-            self.flush()
-            self.buffer.close()
+            try:
+                self.flush()
+            finally:
+                self.buffer.close()
 
     @property
     def closed(self):
index 555ad7444709acb450da31763ef9cf17c613d636..6931e0a5a7dc91ab5b4ab3ea61cbc80c212e926a 100644 (file)
@@ -603,6 +603,7 @@ class IOTest(unittest.TestCase):
             raise IOError()
         f.flush = bad_flush
         self.assertRaises(IOError, f.close) # exception not swallowed
+        self.assertTrue(f.closed)
 
     def test_multi_close(self):
         f = self.open(support.TESTFN, "wb", buffering=0)
@@ -780,6 +781,22 @@ class CommonBufferedTests:
         raw.flush = bad_flush
         b = self.tp(raw)
         self.assertRaises(IOError, b.close) # exception not swallowed
+        self.assertTrue(b.closed)
+
+    def test_close_error_on_close(self):
+        raw = self.MockRawIO()
+        def bad_flush():
+            raise IOError('flush')
+        def bad_close():
+            raise IOError('close')
+        raw.close = bad_close
+        b = self.tp(raw)
+        b.flush = bad_flush
+        with self.assertRaises(IOError) as err: # exception not swallowed
+            b.close()
+        self.assertEqual(err.exception.args, ('close',))
+        self.assertEqual(err.exception.__context__.args, ('flush',))
+        self.assertFalse(b.closed)
 
     def test_multi_close(self):
         raw = self.MockRawIO()
@@ -1296,6 +1313,16 @@ class BufferedWriterTest(unittest.TestCase, CommonBufferedTests):
         with self.assertRaises(TypeError):
             self.tp(self.MockRawIO(), 8, 12)
 
+    def test_write_error_on_close(self):
+        raw = self.MockRawIO()
+        def bad_write(b):
+            raise IOError()
+        raw.write = bad_write
+        b = self.tp(raw)
+        b.write(b'spam')
+        self.assertRaises(IOError, b.close) # exception not swallowed
+        self.assertTrue(b.closed)
+
 
 class CBufferedWriterTest(BufferedWriterTest, SizeofTest):
     tp = io.BufferedWriter
@@ -2465,6 +2492,7 @@ class TextIOWrapperTest(unittest.TestCase):
             raise IOError()
         txt.flush = bad_flush
         self.assertRaises(IOError, txt.close) # exception not swallowed
+        self.assertTrue(txt.closed)
 
     def test_multi_close(self):
         txt = self.TextIOWrapper(self.BytesIO(self.testdata), encoding="ascii")
index 509ea66752615071f3119b1af9f639f668e7ea0f..40e604a36349a9cf1cf574c9c7f0e727f055ad7c 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -12,6 +12,9 @@ What's New in Python 3.3.1?
 Core and Builtins
 -----------------
 
+- Issue #16597: Make BufferedIO.close call close() on the underlying stream if
+  invoking flush() fails.
+
 - Issue #16722: In the bytes() constructor, try to call __bytes__ on the
   argument before __index__.
 
index 334734beccbae8d3d4271eaeb973c46460a620cd..b077f34fff8c49d63031292964880d6fede7a426 100644 (file)
@@ -484,7 +484,7 @@ buffered_closed_get(buffered *self, void *context)
 static PyObject *
 buffered_close(buffered *self, PyObject *args)
 {
-    PyObject *res = NULL;
+    PyObject *res = NULL, *exc = NULL, *val, *tb;
     int r;
 
     CHECK_INITIALIZED(self)
@@ -512,13 +512,29 @@ buffered_close(buffered *self, PyObject *args)
     res = PyObject_CallMethodObjArgs((PyObject *)self, _PyIO_str_flush, NULL);
     if (!ENTER_BUFFERED(self))
         return NULL;
-    if (res == NULL) {
-        goto end;
-    }
-    Py_XDECREF(res);
+    if (res == NULL)
+        PyErr_Fetch(&exc, &val, &tb);
+    else
+        Py_DECREF(res);
 
     res = PyObject_CallMethodObjArgs(self->raw, _PyIO_str_close, NULL);
 
+    if (exc != NULL) {
+        if (res != NULL) {
+            Py_CLEAR(res);
+            PyErr_Restore(exc, val, tb);
+        }
+        else {
+            PyObject *val2;
+            Py_DECREF(exc);
+            Py_XDECREF(tb);
+            PyErr_Fetch(&exc, &val2, &tb);
+            PyErr_NormalizeException(&exc, &val2, &tb);
+            PyException_SetContext(val2, val);
+            PyErr_Restore(exc, val2, tb);
+        }
+    }
+
 end:
     LEAVE_BUFFERED(self)
     return res;
index 96434a81b74a4438201e93cdc44e0cdda0226338..83437d6add847276c30a0ce22a14e1a3d55082a0 100644 (file)
@@ -2554,6 +2554,7 @@ textiowrapper_close(textio *self, PyObject *args)
         Py_RETURN_NONE; /* stream already closed */
     }
     else {
+        PyObject *exc = NULL, *val, *tb;
         if (self->deallocating) {
             res = _PyObject_CallMethodId(self->buffer, &PyId__dealloc_warn, "O", self);
             if (res)
@@ -2562,13 +2563,28 @@ textiowrapper_close(textio *self, PyObject *args)
                 PyErr_Clear();
         }
         res = _PyObject_CallMethodId((PyObject *)self, &PyId_flush, NULL);
-        if (res == NULL) {
-            return NULL;
-        }
+        if (res == NULL)
+            PyErr_Fetch(&exc, &val, &tb);
         else
             Py_DECREF(res);
 
-        return _PyObject_CallMethodId(self->buffer, &PyId_close, NULL);
+        res = _PyObject_CallMethodId(self->buffer, &PyId_close, NULL);
+        if (exc != NULL) {
+            if (res != NULL) {
+                Py_CLEAR(res);
+                PyErr_Restore(exc, val, tb);
+            }
+            else {
+                PyObject *val2;
+                Py_DECREF(exc);
+                Py_XDECREF(tb);
+                PyErr_Fetch(&exc, &val2, &tb);
+                PyErr_NormalizeException(&exc, &val2, &tb);
+                PyException_SetContext(val2, val);
+                PyErr_Restore(exc, val2, tb);
+            }
+        }
+        return res;
     }
 }