gh-120754: Ensure _stat_atopen is cleared on fd change#125166
Conversation
Sorry, something went wrong.
Performed an audit of `fileio.c` and `_pyio` and made sure anytime the fd changes the stat result, if set, is also cleared/changed. There's one case where it's not cleared, if code would clear it in __init__, keep the memory allocated and just do another fstat with the existing memory.
vstinner
left a comment
vstinner
left a comment
There was a problem hiding this comment.
There are compiler warnings that you can see in the review tab.
Sorry, something went wrong.
|
I think the latest commit fixes the compiler warnings (was a testing/debugging assert I accidentally committed with a missing |
Sorry, something went wrong.
|
CI "Tests / Ubuntu SSL tests with OpenSSL" failed because of what looks like a network hang / not these changes: |
Sorry, something went wrong.
|
Merged, thank you. |
Sorry, something went wrong.
…n#125166) Performed an audit of `fileio.c` and `_pyio` and made sure anytime the fd changes the stat result, if set, is also cleared/changed. There's one case where it's not cleared, if code would clear it in __init__, keep the memory allocated and just do another fstat with the existing memory.
…n#125166) Performed an audit of `fileio.c` and `_pyio` and made sure anytime the fd changes the stat result, if set, is also cleared/changed. There's one case where it's not cleared, if code would clear it in __init__, keep the memory allocated and just do another fstat with the existing memory.
While adding _stat_atopen I accidentally created a leak in the C implementation because the
_stat_atopenmember was set to NULL in the C implementation, but the memory wasn't freed. That was resolved, but it showed there were a number of potential cases where_stat_atopenshould be cleared (the information is out of date because the fd changed) that it was not. In this PR I audited by reading through FileIO (in _io and _pyio), and in all cases where the fd is changed/closed made sure to clear/reset _stat_atopen as well.Followup from: #124225 (comment), cc: @vstinner