Skip to content

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

Merged
serhiy-storchaka merged 10 commits into
python:mainfrom
serhiy-storchaka:crash-bytesio-getbuffer-gc
Dec 14, 2023
Merged

gh-111049: Fix crash during garbage collection of the BytesIO buffer object#111221
serhiy-storchaka merged 10 commits into
python:mainfrom
serhiy-storchaka:crash-bytesio-getbuffer-gc

Conversation

@serhiy-storchaka

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

Copy link
Copy Markdown
Member

@serhiy-storchakaserhiy-storchaka added the needs backport to 3.12 only security fixes label Oct 23, 2023
@bedevere-appbedevere-appBot mentioned this pull request Oct 23, 2023
Closed
serhiy-storchaka
serhiy-storchaka commented Oct 23, 2023
View reviewed changes
Comment threadLib/test/test_memoryio.py Outdated
Comment threadLib/test/test_memoryio.py Outdated
Comment threadModules/_io/bytesio.c
erlend-aasland
erlend-aasland requested changes Oct 25, 2023
View reviewed changes
Comment threadLib/test/test_memoryio.py Outdated
@bedevere-app

bedevere-appBot commented Oct 25, 2023

Copy link
Copy Markdown

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

@furkanonderfurkanonder added the type-crash A hard crash of the interpreter, possibly with a core dump label Oct 26, 2023
@serhiy-storchaka

serhiy-storchaka commented Oct 30, 2023

Copy link
Copy Markdown
MemberAuthor

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

@serhiy-storchaka

serhiy-storchaka commented Oct 30, 2023

Copy link
Copy Markdown
MemberAuthor

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

@basepi

basepi commented Oct 30, 2023

Copy link
Copy Markdown

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

AlexWaygood commented Oct 30, 2023

Copy link
Copy Markdown
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
Comment threadLib/test/test_memoryio.py Outdated
Comment threadLib/test/test_memoryio.py Outdated
@basepi

basepi commented Nov 13, 2023

Copy link
Copy Markdown

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

@erlend-aaslanderlend-aasland left a comment

Copy link
Copy Markdown
Contributor

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.

Comment threadLib/test/test_memoryio.py Outdated
Comment threadLib/test/test_memoryio.py Outdated
@basepi

basepi commented Dec 11, 2023

Copy link
Copy Markdown

@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.

@basepibasepi mentioned this pull request Dec 11, 2023
Closed
@serhiy-storchaka
serhiy-storchakaforce-pushed the crash-bytesio-getbuffer-gc branch from 0c693c0 to 621345bCompareDecember 14, 2023 09:44
@serhiy-storchaka

serhiy-storchaka commented Dec 14, 2023

Copy link
Copy Markdown
MemberAuthor

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:mainDec 14, 2023
@miss-islington-app

This comment was marked as outdated.

Sign in to view
@bedevere-app

bedevere-appBot commented Dec 14, 2023

Copy link
Copy Markdown

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-appbedevere-appBot 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)
@basepibasepi 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 freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@erlend-aaslanderlend-aaslanderlend-aasland approved these changes
@kumaraditya303kumaraditya303Awaiting requested review from kumaraditya303

Assignees

No one assigned

Labels

type-crashA 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