From 6be8876623ffab3f2d1e4af46e7ce169d84b35a6 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Mon, 3 May 2010 16:48:20 +0000 Subject: 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. ........ --- Lib/_pyio.py | 22 ++++++++-------------- Lib/test/test_io.py | 44 ++++++++++++++++++++++++++++++++++++++++++++ Misc/NEWS | 4 ++++ Modules/_io/bufferedio.c | 6 +----- Modules/_io/bytesio.c | 1 + Modules/_io/iobase.c | 6 +----- Modules/_io/textio.c | 28 +++++++++++++++++++++------- 7 files changed, 80 insertions(+), 31 deletions(-) diff --git a/Lib/_pyio.py b/Lib/_pyio.py index 1fbefd2..66f990d 100644 --- 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 fa96f9a..aa1679b 100644 --- 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 2cacaf3..35c246b 100644 --- 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 fb651a7..07fe0a1 100644 --- 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 1482d82..4e78cd5 100644 --- 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 271a41e..2d664ab 100644 --- 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 89d922c..b039c2f 100644 --- 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 * -- cgit v0.12