gh-138720: Make Buffered closed check match flush#138724
Uh oh!
There was an error while loading. Please reload this page.
Conversation
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.
For this, I also looked at resolving two other ways:
|
encukou commented Sep 17, 2025
!buildbot PPC64LE.Fedora.Stable.LTO |
bedevere-bot commented Sep 17, 2025
🤖 New build scheduled with the buildbot fleet by @encukou for commit 60fe309 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F138724%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
encukou left a comment
encukou left a comment There was a problem hiding this comment.
Thank you! This looks like a correct change.
| # gh-138720: C BufferedRWPair would destruct in a bad order resulting in | ||
| # an unraisable exception. | ||
| support.gc_collect() | ||
There was a problem hiding this comment.
The test is way more likely to triggered the issue (fixed behavior) with this change.
There was a problem hiding this comment.
On my dev box (64 bit archlinux) adding the explicit GC takes this from "one in a hundred" -> every time
There was a problem hiding this comment.
OK! I couldn't reproduce it myself, but I'll trust the buildbots :)
encukou merged commit db68bfc into python:mainUh oh!
There was an error while loading. Please reload this page.
vstinner commented Sep 18, 2025
Should we backport this bugfix? |
encukou commented Sep 18, 2025
Can you reproduce it on previous versions? If so, please backport :) |
vstinner commented Sep 18, 2025
Oh. I'm unable to reproduce the issue on 3.14. |
cmaloney commented Sep 18, 2025
The code has been the same since the C I/O implementation was added; unless/until someone has an issue on older versions I wouldn't backport. |
4 participants
In
_io__Buffered_flush_implthe macroCHECK_CLOSEDis used to check thebuffered*is in a good state to be flushed. That differs slightly frombuffered_closed.In some cases, that difference would result in
close()(_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.
.readintoresults in unraisable exception #138720