From e93877d141f4214c8e9618b1b241b7aa359f8859 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Tue, 9 Sep 2025 14:33:54 -0700 Subject: [PATCH 1/5] gh-138720: Make Buffered closed check match flush In `_io__Buffered_flush_impl` the macro `CHECK_CLOSED` is used to check the `buffered*` is in a good state to be flushed. That differs slightly from `buffered_closed`. In some cases, that difference would result in `close()` thinking the file needed to be flushed and closed while `flush()` thought the file was already closed. This could happen during GC and would result in an unraisable exception. --- Lib/test/test_io/test_general.py | 4 ++++ Modules/_io/bufferedio.c | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_io/test_general.py b/Lib/test/test_io/test_general.py index 604b56cea21fac..3acae3d07b48f4 100644 --- a/Lib/test/test_io/test_general.py +++ b/Lib/test/test_io/test_general.py @@ -2287,6 +2287,10 @@ def test_readinto(self): self.assertEqual(getattr(pair, method)(data), 5) self.assertEqual(bytes(data), b"abcde") + # gh-138720: C BufferedRWPair would destruct in a bad order resulting in + # an unraisable exception. + support.gc_collect() + def test_write(self): w = self.MockRawIO() pair = self.tp(self.MockRawIO(), w) diff --git a/Modules/_io/bufferedio.c b/Modules/_io/bufferedio.c index 25be21111b95ee..e8765653bb43c2 100644 --- a/Modules/_io/bufferedio.c +++ b/Modules/_io/bufferedio.c @@ -553,8 +553,8 @@ _io__Buffered_close_impl(buffered *self) if (!ENTER_BUFFERED(self)) { return NULL; } - - r = buffered_closed(self); + /* gh-138720: Use IS_CLOSED to match flush CHECK_CLOSED. */ + r = IS_CLOSED(self); if (r < 0) goto end; if (r > 0) { From ef75829e19f94fe44fb218b04c81bee8bc150340 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Tue, 9 Sep 2025 17:59:37 -0700 Subject: [PATCH 2/5] add blurb --- .../Library/2025-09-09-17-57-49.gh-issue-138720.hAtsm-.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2025-09-09-17-57-49.gh-issue-138720.hAtsm-.rst diff --git a/Misc/NEWS.d/next/Library/2025-09-09-17-57-49.gh-issue-138720.hAtsm-.rst b/Misc/NEWS.d/next/Library/2025-09-09-17-57-49.gh-issue-138720.hAtsm-.rst new file mode 100644 index 00000000000000..ebf7ea4df71e6e --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-09-09-17-57-49.gh-issue-138720.hAtsm-.rst @@ -0,0 +1,3 @@ +Fix an issue where :meth:`~BufferedIOBase.close` would think the I/O object was +open and call :meth:`~BufferedIOBase.flush` but flush would error that the +file was already closed by making the two functions use the same logic. From c368fd602c68d08b4f3a5c4e11a02486666a56b1 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Tue, 9 Sep 2025 18:07:12 -0700 Subject: [PATCH 3/5] Fix docs --- .../Library/2025-09-09-17-57-49.gh-issue-138720.hAtsm-.rst | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2025-09-09-17-57-49.gh-issue-138720.hAtsm-.rst b/Misc/NEWS.d/next/Library/2025-09-09-17-57-49.gh-issue-138720.hAtsm-.rst index ebf7ea4df71e6e..92a3e1ccb744e3 100644 --- a/Misc/NEWS.d/next/Library/2025-09-09-17-57-49.gh-issue-138720.hAtsm-.rst +++ b/Misc/NEWS.d/next/Library/2025-09-09-17-57-49.gh-issue-138720.hAtsm-.rst @@ -1,3 +1,4 @@ -Fix an issue where :meth:`~BufferedIOBase.close` would think the I/O object was -open and call :meth:`~BufferedIOBase.flush` but flush would error that the -file was already closed by making the two functions use the same logic. +Fix an issue where the :class:`BufferedIOBase` implementation of +:meth:`~IOBase.close` would think the I/O object was open and call +:meth:`~IOBase.flush` which would error that the file was already closed by +making the two implementations use the same logic. From a9200d332b522cf0d7b29045c7af5af78a6aff00 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Tue, 9 Sep 2025 18:16:32 -0700 Subject: [PATCH 4/5] try, try, try again --- .../Library/2025-09-09-17-57-49.gh-issue-138720.hAtsm-.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2025-09-09-17-57-49.gh-issue-138720.hAtsm-.rst b/Misc/NEWS.d/next/Library/2025-09-09-17-57-49.gh-issue-138720.hAtsm-.rst index 92a3e1ccb744e3..2004796720fdb5 100644 --- a/Misc/NEWS.d/next/Library/2025-09-09-17-57-49.gh-issue-138720.hAtsm-.rst +++ b/Misc/NEWS.d/next/Library/2025-09-09-17-57-49.gh-issue-138720.hAtsm-.rst @@ -1,4 +1,4 @@ -Fix an issue where the :class:`BufferedIOBase` implementation of -:meth:`~IOBase.close` would think the I/O object was open and call -:meth:`~IOBase.flush` which would error that the file was already closed by +Fix an issue where the :class:`io.BufferedIOBase` implementation of +:meth:`~io.IOBase.close` would think the I/O object was open and call +:meth:`~io.IOBase.flush` which would error that the file was already closed by making the two implementations use the same logic. From 60fe309dbb08a36f49e8663809f6713a5b5c0115 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Tue, 9 Sep 2025 21:12:39 -0700 Subject: [PATCH 5/5] news: Doesn't actually touch BufferedIOBase, be more accurate for classes --- .../2025-09-09-17-57-49.gh-issue-138720.hAtsm-.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2025-09-09-17-57-49.gh-issue-138720.hAtsm-.rst b/Misc/NEWS.d/next/Library/2025-09-09-17-57-49.gh-issue-138720.hAtsm-.rst index 2004796720fdb5..4f3f54e1deb89f 100644 --- a/Misc/NEWS.d/next/Library/2025-09-09-17-57-49.gh-issue-138720.hAtsm-.rst +++ b/Misc/NEWS.d/next/Library/2025-09-09-17-57-49.gh-issue-138720.hAtsm-.rst @@ -1,4 +1,4 @@ -Fix an issue where the :class:`io.BufferedIOBase` implementation of -:meth:`~io.IOBase.close` would think the I/O object was open and call -:meth:`~io.IOBase.flush` which would error that the file was already closed by -making the two implementations use the same logic. +Fix an issue where :class:`io.BufferedWriter` and :class:`io.BufferedRandom` +had different definitions of "closed" for :meth:`~io.IOBase.close` and +:meth:`~io.IOBase.flush` which resulted in an exception when close called +flush but flush thought the file was already closed.