Skip to content

gh-143689: Fix BufferedReader.read1 leaving object in reentrant state on error#143690

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom
Merged
vstinner merged 9 commits into from
Jan 19, 2026
Merged

gh-143689: Fix BufferedReader.read1 leaving object in reentrant state on error #143690

vstinner merged 9 commits into from
Jan 19, 2026
+22 −1

Conversation

@hyongtao-code
Copy link
Contributor

@hyongtao-codehyongtao-code commented Jan 11, 2026•
edited by bedevere-app bot
Loading

BufferedReader.read1() could leave the buffered object in a
reentrant (locked) state when an exception was raised while
allocating the output buffer.

This change ensures the internal buffered lock is always released
on error, keeping the object in a consistent state after failures.

@hyongtao-code
Fix BufferedReader.read1 leaving object in reentrant state on error
c59aa74
BufferedReader.read1() could leave the buffered object in a reentrant (locked) state when an exception was raised while allocating the output buffer. This change ensures the internal buffered lock is always released on error, keeping the object in a consistent state after failures. Signed-off-by: Yongtao Huang <[email protected]>
@hyongtao-code
Add test case
b8f4dcf
@bedevere-appbedevere-appbot mentioned this pull request Jan 11, 2026
Open
cmaloney
cmaloney reviewed Jan 11, 2026
View reviewed changes
Copy link
Contributor

@cmaloneycmaloney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch! The fix looks correct. The NEWs entry and test don't quite match the preferred patterns in CPython, left some comments.

@hyongtao-code
Copy link
ContributorAuthor

hyongtao-code commented Jan 11, 2026

The NEWs entry and test don't quite match the preferred patterns in CPython,

Thanks for the review and feedback — I’ve made the requested changes.

sobolevn
sobolevn reviewed Jan 11, 2026
View reviewed changes
@cmaloney
Copy link
Contributor

cmaloney commented Jan 12, 2026

Please don't force-push to CPython PR branches: https://devguide.python.org/getting-started/pull-request-lifecycle/#don-t-force-push

cmaloney
cmaloney approved these changes Jan 12, 2026
View reviewed changes
@hyongtao-code
Copy link
ContributorAuthor

hyongtao-code commented Jan 13, 2026

don't force-push

Sorry — I only realized afterward that using git revert to undo the previous commit would have been the better choice.
Thanks for pointing that out; I’ll keep it in mind.

vstinner
vstinner approved these changes Jan 16, 2026
View reviewed changes
Copy link
Member

@vstinnervstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. It seems like I introduced the regression in 3.15 with the commit 4554486.

@cmaloney
Copy link
Contributor

cmaloney commented Jan 17, 2026

and I missed it in review but should have caught it :)

@vstinnervstinner merged commit 375e372 into python:mainJan 19, 2026
50 checks passed
@vstinner
Copy link
Member

vstinner commented Jan 19, 2026

Merged, thanks for the fix and the test.

@hyongtao-codehyongtao-code deleted the dev-io-buffer branch January 19, 2026 15:50
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cmaloneycmaloneycmaloney approved these changes

@sobolevnsobolevnsobolevn left review comments

@vstinnervstinnervstinner approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@hyongtao-code@cmaloney@vstinner@sobolevn