NotificationsYou must be signed in to change notification settings
Fork33.4k
Star69.8k

gh-138720: Make Buffered closed check match flush#138724

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

encukou merged 5 commits intopython:mainfromcmaloney:bufferedrwpair_gc

Sep 18, 2025

Merged

gh-138720: Make Buffered closed check match flush#138724

encukou merged 5 commits intopython:mainfromcmaloney:bufferedrwpair_gc

Sep 18, 2025

Conversation

Copy link

Contributor

cmaloney commentedSep 10, 2025•
edited
Loading

In_io__Buffered_flush_impl the macroCHECK_CLOSED is used to check thebuffered* is in a good state to be flushed. That differs slightly frombuffered_closed.

In some cases, that difference would result inclose() (_io__Buffered_close_impl) thinking the file needed to be flushed and closed whileflush() thought the file was already closed.

This could happen during GC and would result in an unraisable exception.

Issue:GC of unclosed io.BuffererdRWPair after.readinto results in unraisable exception #138720

pythongh-138720: Make Buffered closed check match flush

e93877d

In `_io__Buffered_flush_impl` the macro `CHECK_CLOSED` is used to checkthe `buffered*` is in a good state to be flushed. That differs slightlyfrom `buffered_closed`.In some cases, that difference would result in `close()` thinking thefile needed to be flushed and closed while `flush()` thought the filewas already closed.This could happen during GC and would result in an unraisable exception.

bedevere-appbot added the awaiting review label

Sep 10, 2025

bedevere-appbot mentioned this pull request

Sep 10, 2025

GC of unclosed io.BuffererdRWPair after.readinto results in unraisable exception#138720

Closed

cmaloney added4 commits

September 9, 2025 17:59

add blurb

ef75829

Fix docs

c368fd6

try, try, try again

a9200d3

news: Doesn't actually touch BufferedIOBase, be more accurate for cla…

60fe309

…sses

This was referencedSep 10, 2025

gh-138013: Move I/O test infrastructre to test_io.utils#138475

Merged

gh-138013: Remove load_tests in test_io.test_general#138771

Merged

Copy link

ContributorAuthor

cmaloney commentedSep 16, 2025•
edited
Loading

For this, I also looked at resolving two other ways:

Updating.closed member: This makes it so.close() isn't called (and therefore.flush()). A lot more tests break with that change, this to me is more minimal fix; BufferedRWPair is not often used (and was thought of being removed once).
Figure out why the GC isn't clearing out in a clean order and resolve: Again fairly intricate to do with a lot more potential unintended consequences.