Skip to content

gh-111049: Fix crash during garbage collection of the BytesIO buffer object #111221

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
merged 10 commits into from
Dec 14, 2023
Merged

gh-111049: Fix crash during garbage collection of the BytesIO buffer object #111221

merged 10 commits into from
Dec 14, 2023
+27 −10

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Oct 23, 2023 •
edited by bedevere-app bot
Loading

@serhiy-storchaka serhiy-storchaka added the needs backport to 3.12 only security fixes label Oct 23, 2023
@bedevere-app bedevere-app bot mentioned this pull request Oct 23, 2023
Closed
serhiy-storchaka
serhiy-storchaka commented Oct 23, 2023
View reviewed changes
erlend-aasland
erlend-aasland requested changes Oct 25, 2023
View reviewed changes
@bedevere-app
Copy link

bedevere-app bot commented Oct 25, 2023

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@furkanonder furkanonder added the type-crash A hard crash of the interpreter, possibly with a core dump label Oct 26, 2023
@serhiy-storchaka
Copy link
Member Author

@erlend-aasland I do not understand, what exactly changes did you request. Could you please explain it in more details?

@serhiy-storchaka
Copy link
Member Author

Thank you for your suggestion @erlend-aasland. I have made the requested changes; please review again.

@basepi
Copy link

basepi commented Oct 30, 2023

I have made the requested changes; please review again.

I think this needs to be in a comment by itself, looks like the bot didn't remove the label.

@AlexWaygood
Copy link
Member

I have made the requested changes; please review again.

I think this needs to be in a comment by itself, looks like the bot didn't remove the label.

no, the bot is malfunctioning at the moment:

erlend-aasland
erlend-aasland reviewed Oct 31, 2023
View reviewed changes
@basepi
Copy link

basepi commented Nov 13, 2023

Any movement here? It would be lovely to get this fix into 3.12.1.

erlend-aasland
erlend-aasland requested changes Nov 15, 2023
View reviewed changes
Copy link
Contributor

@erlend-aasland erlend-aasland left a comment

Choose a reason for hiding this comment

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

Please remove the XXX from the comments as per PEP-8:

Ensure that your comments are clear and easily understandable to other speakers of the language you are writing in.

Python coders from non-English speaking countries: please write your comments in English, unless you are 120% sure that the code will never be read by people who don’t speak your language.

XXX is neither clear, easily understandable, nor english; please remove it.

@basepi
Copy link

basepi commented Dec 11, 2023

@serhiy-storchaka Can you please remove the instances of XXX so we can get this merged? Frustrated that we missed 3.12.1 with this fix.

@basepi basepi mentioned this pull request Dec 11, 2023
Closed
@serhiy-storchaka serhiy-storchaka force-pushed the crash-bytesio-getbuffer-gc branch from 0c693c0 to 621345b Compare December 14, 2023 09:44
@serhiy-storchaka
Copy link
Member Author

I decided to remove them at all. I tried to silence errors, but they serve as indication of programming error, similar to ResourceWarning. When memoryview is explicitly released or used as a context manager, this never happens.

@serhiy-storchaka serhiy-storchaka enabled auto-merge (squash) December 14, 2023 09:51
@serhiy-storchaka serhiy-storchaka merged commit bb36f72 into python:main Dec 14, 2023
@miss-islington-app

This comment was marked as outdated.

Sign in to view
@bedevere-app
Copy link

bedevere-app bot commented Dec 14, 2023

GH-113096 is a backport of this pull request to the 3.12 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 14, 2023
@serhiy-storchaka @miss-islington
pythongh-111049: Fix crash during garbage collection of the BytesIO b…
fc4cf50 
…uffer object (pythonGH-111221)

(cherry picked from commit bb36f72)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Dec 14, 2023
serhiy-storchaka added a commit that referenced this pull request Dec 14, 2023
@miss-islington @serhiy-storchaka
[3.12] gh-111049: Fix crash during garbage collection of the BytesIO …
9f5209f 
…buffer object (GH-111221) (GH-113096)

(cherry picked from commit bb36f72)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
corona10 pushed a commit to corona10/cpython that referenced this pull request Dec 15, 2023
@serhiy-storchaka @corona10
pythongh-111049: Fix crash during garbage collection of the BytesIO b…
43fea31 
…uffer object (pythonGH-111221)
@basepi basepi mentioned this pull request Jan 18, 2024
Closed
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
@serhiy-storchaka @aisk
pythongh-111049: Fix crash during garbage collection of the BytesIO b…
db183c4 
…uffer object (pythonGH-111221)
@serhiy-storchaka serhiy-storchaka deleted the crash-bytesio-getbuffer-gc branch March 12, 2024 10:11
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@serhiy-storchaka @Glyphack
pythongh-111049: Fix crash during garbage collection of the BytesIO b…
2a1f6ae 
…uffer object (pythonGH-111221)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erlend-aasland erlend-aasland erlend-aasland approved these changes

@kumaraditya303 kumaraditya303 Awaiting requested review from kumaraditya303

Assignees
No one assigned
Labels
type-crash A hard crash of the interpreter, possibly with a core dump
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@serhiy-storchaka @basepi @AlexWaygood @erlend-aasland @furkanonder