From dacd3c1b38845a1a13fd8da1dfe0a66b5c5f314f Mon Sep 17 00:00:00 2001 From: inventshah <39803835+inventshah@users.noreply.github.com> Date: Mon, 27 Oct 2025 00:45:03 -0400 Subject: [PATCH 1/7] preserve sign of IS_CLOSED(...) --- Lib/test/test_io/test_bufferedio.py | 8 ++++++++ .../2025-10-27-00-40-49.gh-issue-140650.DYJPJ9.rst | 2 ++ Modules/_io/bufferedio.c | 2 +- 3 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Library/2025-10-27-00-40-49.gh-issue-140650.DYJPJ9.rst diff --git a/Lib/test/test_io/test_bufferedio.py b/Lib/test/test_io/test_bufferedio.py index 6e9e96b0e55262..62da16aa95dd8e 100644 --- a/Lib/test/test_io/test_bufferedio.py +++ b/Lib/test/test_io/test_bufferedio.py @@ -922,6 +922,14 @@ def test_slow_close_from_thread(self): self.assertTrue(bufio.closed) t.join() + def test_close_without_closed(self): + # gh-140650: check TypeError is raised + class MockRawIOWithoutClosed(self.MockRawIO): + closed = NotImplemented + + bufio = self.tp(MockRawIOWithoutClosed()) + self.assertRaises(TypeError, bufio.close) + class CBufferedWriterTest(BufferedWriterTest, SizeofTest, CTestCase): tp = io.BufferedWriter diff --git a/Misc/NEWS.d/next/Library/2025-10-27-00-40-49.gh-issue-140650.DYJPJ9.rst b/Misc/NEWS.d/next/Library/2025-10-27-00-40-49.gh-issue-140650.DYJPJ9.rst new file mode 100644 index 00000000000000..3e4f34cb077be2 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-10-27-00-40-49.gh-issue-140650.DYJPJ9.rst @@ -0,0 +1,2 @@ +Fix an issue where :meth:`io.BufferedWriter.close` would crash when "closed" +was not implemented. diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index 0a2b35025321cf..0ffed3d726da81 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -362,7 +362,7 @@ _enter_buffered_busy(buffered *self) } #define IS_CLOSED(self) \ - (!self->buffer || \ + (!self->buffer ?: \ (self->fast_closed_checks \ ? _PyFileIO_closed(self->raw) \ : buffered_closed(self))) From 409b9a944bc4618064381f911fdfd5b4c630c018 Mon Sep 17 00:00:00 2001 From: inventshah <39803835+inventshah@users.noreply.github.com> Date: Mon, 27 Oct 2025 01:16:16 -0400 Subject: [PATCH 2/7] Fix ternary, doc method ref, move test to just c impl --- Lib/test/test_io/test_bufferedio.py | 16 ++++++++-------- ...025-10-27-00-40-49.gh-issue-140650.DYJPJ9.rst | 4 ++-- Modules/_io/bufferedio.c | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/Lib/test/test_io/test_bufferedio.py b/Lib/test/test_io/test_bufferedio.py index 62da16aa95dd8e..1c93f347385409 100644 --- a/Lib/test/test_io/test_bufferedio.py +++ b/Lib/test/test_io/test_bufferedio.py @@ -922,14 +922,6 @@ def test_slow_close_from_thread(self): self.assertTrue(bufio.closed) t.join() - def test_close_without_closed(self): - # gh-140650: check TypeError is raised - class MockRawIOWithoutClosed(self.MockRawIO): - closed = NotImplemented - - bufio = self.tp(MockRawIOWithoutClosed()) - self.assertRaises(TypeError, bufio.close) - class CBufferedWriterTest(BufferedWriterTest, SizeofTest, CTestCase): tp = io.BufferedWriter @@ -970,6 +962,14 @@ def test_args_error(self): with self.assertRaisesRegex(TypeError, "BufferedWriter"): self.tp(self.BytesIO(), 1024, 1024, 1024) + def test_close_without_closed(self): + # gh-140650: check TypeError is raised + class MockRawIOWithoutClosed(self.MockRawIO): + closed = NotImplemented + + bufio = self.tp(MockRawIOWithoutClosed()) + self.assertRaises(TypeError, bufio.close) + class PyBufferedWriterTest(BufferedWriterTest, PyTestCase): tp = pyio.BufferedWriter diff --git a/Misc/NEWS.d/next/Library/2025-10-27-00-40-49.gh-issue-140650.DYJPJ9.rst b/Misc/NEWS.d/next/Library/2025-10-27-00-40-49.gh-issue-140650.DYJPJ9.rst index 3e4f34cb077be2..924888e2a277c7 100644 --- a/Misc/NEWS.d/next/Library/2025-10-27-00-40-49.gh-issue-140650.DYJPJ9.rst +++ b/Misc/NEWS.d/next/Library/2025-10-27-00-40-49.gh-issue-140650.DYJPJ9.rst @@ -1,2 +1,2 @@ -Fix an issue where :meth:`io.BufferedWriter.close` would crash when "closed" -was not implemented. +Fix an issue where closing :class:`io.BufferedWriter` could crash +if the closed attribute had the wrong type. diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index 0ffed3d726da81..5497c3d0d9413c 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -362,7 +362,7 @@ _enter_buffered_busy(buffered *self) } #define IS_CLOSED(self) \ - (!self->buffer ?: \ + (!self->buffer ? !self->buffer : \ (self->fast_closed_checks \ ? _PyFileIO_closed(self->raw) \ : buffered_closed(self))) From 0e5bed9d788e63b1f541c281534656659491afe8 Mon Sep 17 00:00:00 2001 From: inventshah <39803835+inventshah@users.noreply.github.com> Date: Mon, 27 Oct 2025 09:17:34 -0400 Subject: [PATCH 3/7] swap from ?: to PyErr_Occurred --- Modules/_io/bufferedio.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index 5497c3d0d9413c..9279925ce37755 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -362,7 +362,7 @@ _enter_buffered_busy(buffered *self) } #define IS_CLOSED(self) \ - (!self->buffer ? !self->buffer : \ + (!self->buffer || \ (self->fast_closed_checks \ ? _PyFileIO_closed(self->raw) \ : buffered_closed(self))) @@ -555,10 +555,9 @@ _io__Buffered_close_impl(buffered *self) } /* gh-138720: Use IS_CLOSED to match flush CHECK_CLOSED. */ r = IS_CLOSED(self); - if (r < 0) - goto end; if (r > 0) { - res = Py_NewRef(Py_None); + if (!PyErr_Occurred()) + res = Py_NewRef(Py_None); goto end; } From 61122b117a51cf4cd929754626ecabef02f99e9d Mon Sep 17 00:00:00 2001 From: inventshah <39803835+inventshah@users.noreply.github.com> Date: Mon, 27 Oct 2025 18:44:00 -0400 Subject: [PATCH 4/7] apply feedback: allow IS_CLOSED to return -1 for error checking --- Lib/test/test_io/test_bufferedio.py | 3 +- ...-10-27-00-40-49.gh-issue-140650.DYJPJ9.rst | 2 +- Modules/_io/bufferedio.c | 31 ++++++++++++++----- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/Lib/test/test_io/test_bufferedio.py b/Lib/test/test_io/test_bufferedio.py index 1c93f347385409..3eb25274e8e64a 100644 --- a/Lib/test/test_io/test_bufferedio.py +++ b/Lib/test/test_io/test_bufferedio.py @@ -962,12 +962,13 @@ def test_args_error(self): with self.assertRaisesRegex(TypeError, "BufferedWriter"): self.tp(self.BytesIO(), 1024, 1024, 1024) - def test_close_without_closed(self): + def test_closed_errors(self): # gh-140650: check TypeError is raised class MockRawIOWithoutClosed(self.MockRawIO): closed = NotImplemented bufio = self.tp(MockRawIOWithoutClosed()) + self.assertRaises(TypeError, bufio.write, b"") self.assertRaises(TypeError, bufio.close) diff --git a/Misc/NEWS.d/next/Library/2025-10-27-00-40-49.gh-issue-140650.DYJPJ9.rst b/Misc/NEWS.d/next/Library/2025-10-27-00-40-49.gh-issue-140650.DYJPJ9.rst index 924888e2a277c7..6765f86825e043 100644 --- a/Misc/NEWS.d/next/Library/2025-10-27-00-40-49.gh-issue-140650.DYJPJ9.rst +++ b/Misc/NEWS.d/next/Library/2025-10-27-00-40-49.gh-issue-140650.DYJPJ9.rst @@ -1,2 +1,2 @@ Fix an issue where closing :class:`io.BufferedWriter` could crash -if the closed attribute had the wrong type. +if the closed attribute raised an exception. diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index 9279925ce37755..fce6c6bd351f1e 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -362,16 +362,24 @@ _enter_buffered_busy(buffered *self) } #define IS_CLOSED(self) \ - (!self->buffer || \ + (!self->buffer ? !self->buffer : \ (self->fast_closed_checks \ ? _PyFileIO_closed(self->raw) \ : buffered_closed(self))) #define CHECK_CLOSED(self, error_msg) \ - if (IS_CLOSED(self) && (Py_SAFE_DOWNCAST(READAHEAD(self), Py_off_t, Py_ssize_t) == 0)) { \ - PyErr_SetString(PyExc_ValueError, error_msg); \ - return NULL; \ - } \ + do { \ + int _closed = IS_CLOSED(self); \ + if (_closed < 0) { \ + return NULL; \ + } \ + if (_closed && \ + (Py_SAFE_DOWNCAST(READAHEAD(self), Py_off_t, Py_ssize_t) == 0)) \ + { \ + PyErr_SetString(PyExc_ValueError, error_msg); \ + return NULL; \ + } \ + } while (0); #define VALID_READ_BUFFER(self) \ (self->readable && self->read_end != -1) @@ -555,9 +563,11 @@ _io__Buffered_close_impl(buffered *self) } /* gh-138720: Use IS_CLOSED to match flush CHECK_CLOSED. */ r = IS_CLOSED(self); + if (r < 0) { + goto end; + } if (r > 0) { - if (!PyErr_Occurred()) - res = Py_NewRef(Py_None); + res = Py_None; goto end; } @@ -2078,6 +2088,7 @@ _io_BufferedWriter_write_impl(buffered *self, Py_buffer *buffer) PyObject *res = NULL; Py_ssize_t written, avail, remaining; Py_off_t offset; + int r; CHECK_INITIALIZED(self) @@ -2086,7 +2097,11 @@ _io_BufferedWriter_write_impl(buffered *self, Py_buffer *buffer) /* Issue #31976: Check for closed file after acquiring the lock. Another thread could be holding the lock while closing the file. */ - if (IS_CLOSED(self)) { + r = IS_CLOSED(self); + if (r < 0) { + goto error; + } + if (r > 0) { PyErr_SetString(PyExc_ValueError, "write to closed file"); goto error; } From 096c2a36745586be1169a3057751600ad687739f Mon Sep 17 00:00:00 2001 From: inventshah <39803835+inventshah@users.noreply.github.com> Date: Mon, 27 Oct 2025 20:28:26 -0400 Subject: [PATCH 5/7] add .flush() test case --- Lib/test/test_io/test_bufferedio.py | 1 + Modules/_io/bufferedio.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_io/test_bufferedio.py b/Lib/test/test_io/test_bufferedio.py index 3eb25274e8e64a..7f13a8fa6495d1 100644 --- a/Lib/test/test_io/test_bufferedio.py +++ b/Lib/test/test_io/test_bufferedio.py @@ -969,6 +969,7 @@ class MockRawIOWithoutClosed(self.MockRawIO): bufio = self.tp(MockRawIOWithoutClosed()) self.assertRaises(TypeError, bufio.write, b"") + self.assertRaises(TypeError, bufio.flush) self.assertRaises(TypeError, bufio.close) diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index fce6c6bd351f1e..b8601762f40128 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -362,7 +362,7 @@ _enter_buffered_busy(buffered *self) } #define IS_CLOSED(self) \ - (!self->buffer ? !self->buffer : \ + (!self->buffer ? 1 : \ (self->fast_closed_checks \ ? _PyFileIO_closed(self->raw) \ : buffered_closed(self))) From 6e70bfe2a05906face6fcd60a2076e8d46db754b Mon Sep 17 00:00:00 2001 From: inventshah <39803835+inventshah@users.noreply.github.com> Date: Wed, 29 Oct 2025 22:15:23 -0400 Subject: [PATCH 6/7] revert unrelated changes --- Modules/_io/bufferedio.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index b8601762f40128..0b4bc4c6b8ad42 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -563,11 +563,10 @@ _io__Buffered_close_impl(buffered *self) } /* gh-138720: Use IS_CLOSED to match flush CHECK_CLOSED. */ r = IS_CLOSED(self); - if (r < 0) { + if (r < 0) goto end; - } if (r > 0) { - res = Py_None; + res = Py_NewRef(Py_None); goto end; } From e405525b18c1666aa75d09fe79cdf25b56779b3f Mon Sep 17 00:00:00 2001 From: inventshah <39803835+inventshah@users.noreply.github.com> Date: Wed, 29 Oct 2025 22:30:46 -0400 Subject: [PATCH 7/7] add test case --- Lib/test/test_io/test_bufferedio.py | 13 ++++++++++++- .../2025-10-27-00-40-49.gh-issue-140650.DYJPJ9.rst | 5 +++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_io/test_bufferedio.py b/Lib/test/test_io/test_bufferedio.py index 7f13a8fa6495d1..30c34e818b1572 100644 --- a/Lib/test/test_io/test_bufferedio.py +++ b/Lib/test/test_io/test_bufferedio.py @@ -962,7 +962,7 @@ def test_args_error(self): with self.assertRaisesRegex(TypeError, "BufferedWriter"): self.tp(self.BytesIO(), 1024, 1024, 1024) - def test_closed_errors(self): + def test_non_boolean_closed_attr(self): # gh-140650: check TypeError is raised class MockRawIOWithoutClosed(self.MockRawIO): closed = NotImplemented @@ -972,6 +972,17 @@ class MockRawIOWithoutClosed(self.MockRawIO): self.assertRaises(TypeError, bufio.flush) self.assertRaises(TypeError, bufio.close) + def test_closed_attr_raises(self): + class MockRawIOClosedRaises(self.MockRawIO): + @property + def closed(self): + raise ValueError("test") + + bufio = self.tp(MockRawIOClosedRaises()) + self.assertRaisesRegex(ValueError, "test", bufio.write, b"") + self.assertRaisesRegex(ValueError, "test", bufio.flush) + self.assertRaisesRegex(ValueError, "test", bufio.close) + class PyBufferedWriterTest(BufferedWriterTest, PyTestCase): tp = pyio.BufferedWriter diff --git a/Misc/NEWS.d/next/Library/2025-10-27-00-40-49.gh-issue-140650.DYJPJ9.rst b/Misc/NEWS.d/next/Library/2025-10-27-00-40-49.gh-issue-140650.DYJPJ9.rst index 6765f86825e043..2ae153a64808e8 100644 --- a/Misc/NEWS.d/next/Library/2025-10-27-00-40-49.gh-issue-140650.DYJPJ9.rst +++ b/Misc/NEWS.d/next/Library/2025-10-27-00-40-49.gh-issue-140650.DYJPJ9.rst @@ -1,2 +1,3 @@ -Fix an issue where closing :class:`io.BufferedWriter` could crash -if the closed attribute raised an exception. +Fix an issue where closing :class:`io.BufferedWriter` could crash if +the closed attribute raised an exception on access or could not be +converted to a boolean.