Merged revisions 80720 via svnmerge from

svn+ssh://pythondev@svn.python.org/python/trunk

........
  r80720 | antoine.pitrou | 2010-05-03 18:25:33 +0200 (lun., 03 mai 2010) | 5 lines

  Issue #7865: The close() method of :mod:`io` objects should not swallow
  exceptions raised by the implicit flush().  Also ensure that calling
  close() several times is supported.  Patch by Pascal Chambon.
........

diff --git a/Lib/_pyio.py b/Lib/_pyio.py
index 1fbefd239f3001c127dc94dd176b83d0e8a49035..66f990d79b41b9572bd9358e0f72ffb178dfc95b 100644 (file)
--- a/Lib/_pyio.py
+++ b/Lib/_pyio.py
@@ -322,6 +322,7 @@ class IOBase(metaclass=abc.ABCMeta):
 
         This is not implemented for read-only and non-blocking streams.
         """
+        self._checkClosed()
         # XXX Should this return the number of bytes written???
 
     __closed = False
@@ -332,10 +333,7 @@ class IOBase(metaclass=abc.ABCMeta):
         This method has no effect if the file is already closed.
         """
         if not self.__closed:
-            try:
-                self.flush()
-            except IOError:
-                pass  # If flush() fails, just give up
+            self.flush()
             self.__closed = True
 
     def __del__(self) -> None:
@@ -702,14 +700,13 @@ class _BufferedIOMixin(BufferedIOBase):
     ### Flush and close ###
 
     def flush(self):
+        if self.closed:
+            raise ValueError("flush of closed file")
         self.raw.flush()
 
     def close(self):
-        if not self.closed and self.raw is not None:
-            try:
-                self.flush()
-            except IOError:
-                pass  # If flush() fails, just give up
+        if self.raw is not None and not self.closed:
+            self.flush()
             self.raw.close()
 
     def detach(self):
@@ -1521,11 +1518,8 @@ class TextIOWrapper(TextIOBase):
         self._telling = self._seekable
 
     def close(self):
-        if self.buffer is not None:
-            try:
-                self.flush()
-            except IOError:
-                pass  # If flush() fails, just give up
+        if self.buffer is not None and not self.closed:
+            self.flush()
             self.buffer.close()
 
     @property

diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py
index fa96f9a600936cf8d3074686744dc38a011b59a2..aa1679b973222b3267979854a025dcb73a0c7d4b 100644 (file)
--- a/Lib/test/test_io.py
+++ b/Lib/test/test_io.py
@@ -536,6 +536,20 @@ class IOTest(unittest.TestCase):
         with self.open(zero, "r") as f:
             self.assertRaises(OverflowError, f.read)
 
+    def test_flush_error_on_close(self):
+        f = self.open(support.TESTFN, "wb", buffering=0)
+        def bad_flush():
+            raise IOError()
+        f.flush = bad_flush
+        self.assertRaises(IOError, f.close) # exception not swallowed
+
+    def test_multi_close(self):
+        f = self.open(support.TESTFN, "wb", buffering=0)
+        f.close()
+        f.close()
+        f.close()
+        self.assertRaises(ValueError, f.flush)
+
 class CIOTest(IOTest):
     pass
 
@@ -635,6 +649,22 @@ class CommonBufferedTests:
         raw.name = b"dummy"
         self.assertEqual(repr(b), "<%s name=b'dummy'>" % clsname)
 
+    def test_flush_error_on_close(self):
+        raw = self.MockRawIO()
+        def bad_flush():
+            raise IOError()
+        raw.flush = bad_flush
+        b = self.tp(raw)
+        self.assertRaises(IOError, b.close) # exception not swallowed
+
+    def test_multi_close(self):
+        raw = self.MockRawIO()
+        b = self.tp(raw)
+        b.close()
+        b.close()
+        b.close()
+        self.assertRaises(ValueError, b.flush)
+
 
 class BufferedReaderTest(unittest.TestCase, CommonBufferedTests):
     read_mode = "rb"
@@ -2107,6 +2137,20 @@ class TextIOWrapperTest(unittest.TestCase):
             for n in range(20):
                 self.assertEquals(content.count("Thread%03d\n" % n), 1)
 
+    def test_flush_error_on_close(self):
+        txt = self.TextIOWrapper(self.BytesIO(self.testdata), encoding="ascii")
+        def bad_flush():
+            raise IOError()
+        txt.flush = bad_flush
+        self.assertRaises(IOError, txt.close) # exception not swallowed
+
+    def test_multi_close(self):
+        txt = self.TextIOWrapper(self.BytesIO(self.testdata), encoding="ascii")
+        txt.close()
+        txt.close()
+        txt.close()
+        self.assertRaises(ValueError, txt.flush)
+
 class CTextIOWrapperTest(TextIOWrapperTest):
 
     def test_initialization(self):

diff --git a/Misc/NEWS b/Misc/NEWS
index 2cacaf325630a7ad9a17042dcf00bba3bcf817df..35c246bdb9842a8dac647f0d248e17a12c53f068 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -345,6 +345,10 @@ C-API
 Library
 -------
 
+- Issue #7865: The close() method of :mod:`io` objects should not swallow
+  exceptions raised by the implicit flush().  Also ensure that calling
+  close() several times is supported.  Patch by Pascal Chambon.
+
 - Issue #4687: Fix accuracy of garbage collection runtimes displayed with
   gc.DEBUG_STATS.
 

diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c
index fb651a7f9a0afa4777deb25dc11381f32450c8b2..07fe0a1d81d63c6cb5ba77fdda31db03a6e9b89a 100644 (file)
--- a/Modules/_io/bufferedio.c
+++ b/Modules/_io/bufferedio.c
@@ -440,11 +440,7 @@ buffered_close(buffered *self, PyObject *args)
     res = PyObject_CallMethodObjArgs((PyObject *)self, _PyIO_str_flush, NULL);
     ENTER_BUFFERED(self)
     if (res == NULL) {
-        /* If flush() fails, just give up */
-        if (PyErr_ExceptionMatches(PyExc_IOError))
-            PyErr_Clear();
-        else
-            goto end;
+        goto end;
     }
     Py_XDECREF(res);
 

diff --git a/Modules/_io/bytesio.c b/Modules/_io/bytesio.c
index 1482d824f12504ad6d0ad367231c1364f2e32a18..4e78cd52639ee57c6fcdffcabfd13c310f69b450 100644 (file)
--- a/Modules/_io/bytesio.c
+++ b/Modules/_io/bytesio.c
@@ -169,6 +169,7 @@ PyDoc_STRVAR(flush_doc,
 static PyObject *
 bytesio_flush(bytesio *self)
 {
+    CHECK_CLOSED(self);
     Py_RETURN_NONE;
 }
 

diff --git a/Modules/_io/iobase.c b/Modules/_io/iobase.c
index 271a41efba79b213eff79a8ca9982a2f1a1debcc..2d664ab48c90154063d398fb3c47c1bc562e243d 100644 (file)
--- a/Modules/_io/iobase.c
+++ b/Modules/_io/iobase.c
@@ -183,11 +183,7 @@ iobase_close(PyObject *self, PyObject *args)
     res = PyObject_CallMethodObjArgs(self, _PyIO_str_flush, NULL);
     PyObject_SetAttrString(self, "__IOBase_closed", Py_True);
     if (res == NULL) {
-        /* If flush() fails, just give up */
-        if (PyErr_ExceptionMatches(PyExc_IOError))
-            PyErr_Clear();
-        else
-            return NULL;
+        return NULL;
     }
     Py_XDECREF(res);
     Py_RETURN_NONE;

diff --git a/Modules/_io/textio.c b/Modules/_io/textio.c
index 89d922c351897014aed6803cc50e7c85e348228b..b039c2f6a38184f42c02723c17665226e65ae691 100644 (file)
--- a/Modules/_io/textio.c
+++ b/Modules/_io/textio.c
@@ -2398,16 +2398,30 @@ static PyObject *
 textiowrapper_close(textio *self, PyObject *args)
 {
     PyObject *res;
+    int r;
     CHECK_INITIALIZED(self);
-    res = PyObject_CallMethod((PyObject *)self, "flush", NULL);
-    if (res == NULL) {
-        /* If flush() fails, just give up */
-        PyErr_Clear();
+
+    res = textiowrapper_closed_get(self, NULL);
+    if (res == NULL)
+        return NULL;
+    r = PyObject_IsTrue(res);
+    Py_DECREF(res);
+    if (r < 0)
+        return NULL;
+    
+    if (r > 0) {
+        Py_RETURN_NONE; /* stream already closed */
     }
-    else
-        Py_DECREF(res);
+    else {
+        res = PyObject_CallMethod((PyObject *)self, "flush", NULL);
+        if (res == NULL) {
+            return NULL;
+        }
+        else
+            Py_DECREF(res);
 
-    return PyObject_CallMethod(self->buffer, "close", NULL);
+        return PyObject_CallMethod(self->buffer, "close", NULL);
+    }
 }
 
 static PyObject *