gh-138720: Make Buffered closed check match flush #138724
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:
|
|
!buildbot PPC64LE.Fedora.Stable.LTO |
|
🤖 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:
|
| # 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 :)
|
Should we backport this bugfix? |
|
Can you reproduce it on previous versions? If so, please backport :) |
Oh. I'm unable to reproduce the issue on 3.14. |
|
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. |
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