gh-126341: added release check to iteration method of memoryview object by ritvikpasham · Pull Request #126759 · python/cpython

ritvikpasham · 2024-11-12T19:17:55Z

Issue: SystemError from calling __iter__ on a released memoryview #126341

ghost · 2024-11-12T19:17:59Z

All commit authors signed the Contributor License Agreement.

bedevere-app · 2024-11-12T19:18:00Z

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

sobolevn

Please, add a test case.

bedevere-app · 2024-11-12T19:39:21Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be put in the comfy chair!

ZeroIntensity

Thanks for contributing :)

You should be able to use the reproducer from the issue as a test case.

Misc/NEWS.d/next/Core_and_Builtins/2024-11-12-19-24-00.gh-issue-126341.5SdAe1.rst

…to fix-issue-126341

…e-126341.5SdAe1.rst Co-authored-by: Peter Bierma <zintensitydev@gmail.com>

ritvikpasham · 2024-11-12T20:18:49Z

I have made the requested changes; please review again.

bedevere-app · 2024-11-12T20:18:54Z

Thanks for making the requested changes!

@sobolevn: please review the changes made to this pull request.

…to fix-issue-126341

ZeroIntensity

I don't think we need a whole new file for the test, as it's only a 3 line reproducer. You could just add it alongside the existing tests for memoryview iteration.

Lib/test/memoryview/test_released_iter.py

ritvikpasham · 2024-11-12T20:32:36Z

I don't think we need a whole new file for the test, as it's only a 3 line reproducer. You could just add it alongside the existing tests for memoryview iteration.

Yeah, that makes sense! Would you happen to know where the memoryview iteration tests are?

ZeroIntensity · 2024-11-12T20:36:51Z

I don't, but you can just CTRL+F iter in the memoryview tests and you'll probably find it :)

… tests

ritvikpasham · 2024-11-12T21:13:10Z

It look like there was a previous test case that tests CHECK_RELEASED, so I just added a check to the iter method there.

ZeroIntensity

Thank you!

Misc/NEWS.d/next/Core_and_Builtins/2024-11-12-19-24-00.gh-issue-126341.5SdAe1.rst

…e-126341.5SdAe1.rst Co-authored-by: sobolevn <mail@sobolevn.me>

vstinner

LGTM. Thanks for adding a non-regression test!

vstinner · 2024-11-13T11:20:39Z

@sobolevn: I let you review the updated PR. You can merge it if it looks good to you.

sobolevn

Thanks!

sobolevn · 2024-11-13T11:43:37Z

Should we backport this change?

ZeroIntensity · 2024-11-13T11:44:34Z

I think so, it's a bugfix.

miss-islington-app · 2024-11-13T11:51:05Z

Thanks @ritvikpasham for the PR, and @sobolevn for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

…w` (pythonGH-126759) (cherry picked from commit a12690e) Co-authored-by: Ritvik Pasham <ritvikpasham@gmail.com> Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com> Co-authored-by: Peter Bierma <zintensitydev@gmail.com> Co-authored-by: Victor Stinner <vstinner@python.org> Co-authored-by: sobolevn <mail@sobolevn.me>

bedevere-app · 2024-11-13T11:51:18Z

GH-126778 is a backport of this pull request to the 3.13 branch.

…w` (pythonGH-126759) (cherry picked from commit a12690e) Co-authored-by: Ritvik Pasham <ritvikpasham@gmail.com> Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com> Co-authored-by: Peter Bierma <zintensitydev@gmail.com> Co-authored-by: Victor Stinner <vstinner@python.org> Co-authored-by: sobolevn <mail@sobolevn.me>

bedevere-app · 2024-11-13T11:51:24Z

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

ZeroIntensity · 2024-11-13T11:52:05Z

Congrats on your first contribution @ritvikpasham 🎉

…ew` (GH-126759) (#126779) gh-126341: add release check to `__iter__` method of `memoryview` (GH-126759) (cherry picked from commit a12690e) Co-authored-by: Ritvik Pasham <ritvikpasham@gmail.com> Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com> Co-authored-by: Peter Bierma <zintensitydev@gmail.com> Co-authored-by: Victor Stinner <vstinner@python.org> Co-authored-by: sobolevn <mail@sobolevn.me>

…ew` (GH-126759) (#126778) gh-126341: add release check to `__iter__` method of `memoryview` (GH-126759) (cherry picked from commit a12690e) Co-authored-by: Ritvik Pasham <ritvikpasham@gmail.com> Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com> Co-authored-by: Peter Bierma <zintensitydev@gmail.com> Co-authored-by: Victor Stinner <vstinner@python.org> Co-authored-by: sobolevn <mail@sobolevn.me>

…w` (python#126759) Co-authored-by: blurb-it[bot] <43283697+blurb-it[bot]@users.noreply.github.com> Co-authored-by: Peter Bierma <zintensitydev@gmail.com> Co-authored-by: Victor Stinner <vstinner@python.org> Co-authored-by: sobolevn <mail@sobolevn.me>

Added release check to iteration method of memoryview object

17d417b

bedevere-app bot added the awaiting review label Nov 12, 2024

bedevere-app bot mentioned this pull request Nov 12, 2024

SystemError from calling __iter__ on a released memoryview #126341

Closed

📜🤖 Added by blurb_it.

d456e18

sobolevn requested changes Nov 12, 2024

View reviewed changes

bedevere-app bot removed the awaiting review label Nov 12, 2024

bedevere-app bot added the awaiting changes label Nov 12, 2024

ZeroIntensity reviewed Nov 12, 2024

View reviewed changes

Misc/NEWS.d/next/Core_and_Builtins/2024-11-12-19-24-00.gh-issue-126341.5SdAe1.rst Outdated Show resolved Hide resolved

ritvikpasham and others added 3 commits November 12, 2024 15:13

Added testcase for added release check

abc5088

Merge branch 'fix-issue-126341' of github.com:ritvikpasham/cpython in…

9f7d4b1

…to fix-issue-126341

Update Misc/NEWS.d/next/Core_and_Builtins/2024-11-12-19-24-00.gh-issu…

c50fec3

…e-126341.5SdAe1.rst Co-authored-by: Peter Bierma <zintensitydev@gmail.com>

bedevere-app bot added awaiting change review and removed awaiting changes labels Nov 12, 2024

bedevere-app bot requested a review from sobolevn November 12, 2024 20:18

ritvikpasham added 2 commits November 12, 2024 15:19

Fixed linting issue

5d857d1

Merge branch 'fix-issue-126341' of github.com:ritvikpasham/cpython in…

de3592e

…to fix-issue-126341

ZeroIntensity reviewed Nov 12, 2024

View reviewed changes

Lib/test/memoryview/test_released_iter.py Outdated Show resolved Hide resolved

Corrected trailing whitespace

17db5b4

added release check on iter to memoryview object and deleted previous…

61cae74

… tests

ZeroIntensity approved these changes Nov 12, 2024

View reviewed changes

sobolevn reviewed Nov 13, 2024

View reviewed changes

Misc/NEWS.d/next/Core_and_Builtins/2024-11-12-19-24-00.gh-issue-126341.5SdAe1.rst Outdated Show resolved Hide resolved

Update Misc/NEWS.d/next/Core_and_Builtins/2024-11-12-19-24-00.gh-issu…

a1fe120

…e-126341.5SdAe1.rst Co-authored-by: sobolevn <mail@sobolevn.me>

vstinner approved these changes Nov 13, 2024

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting change review labels Nov 13, 2024

vstinner added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Nov 13, 2024

sobolevn approved these changes Nov 13, 2024

View reviewed changes

sobolevn enabled auto-merge (squash) November 13, 2024 11:39

sobolevn merged commit a12690e into python:main Nov 13, 2024

bedevere-app bot removed the awaiting merge label Nov 13, 2024

bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Nov 13, 2024

bedevere-app bot removed the needs backport to 3.12 only security fixes label Nov 13, 2024

Uh oh!

Conversation

ritvikpasham commented Nov 12, 2024 • edited by bedevere-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Nov 12, 2024 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-app bot commented Nov 12, 2024

Uh oh!

sobolevn left a comment

Choose a reason for hiding this comment

Uh oh!

bedevere-app bot commented Nov 12, 2024

Uh oh!

ZeroIntensity left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ritvikpasham commented Nov 12, 2024

Uh oh!

bedevere-app bot commented Nov 12, 2024

Uh oh!

ZeroIntensity left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ritvikpasham commented Nov 12, 2024

Uh oh!

ZeroIntensity commented Nov 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ritvikpasham commented Nov 12, 2024

Uh oh!

ZeroIntensity left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

vstinner commented Nov 13, 2024

Uh oh!

sobolevn left a comment

Choose a reason for hiding this comment

Uh oh!

sobolevn commented Nov 13, 2024

Uh oh!

ZeroIntensity commented Nov 13, 2024

Uh oh!

miss-islington-app bot commented Nov 13, 2024

Uh oh!

bedevere-app bot commented Nov 13, 2024

Uh oh!

bedevere-app bot commented Nov 13, 2024

Uh oh!

ZeroIntensity commented Nov 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ritvikpasham commented Nov 12, 2024 •

edited by bedevere-app bot

Loading

ghost commented Nov 12, 2024 •

edited by ghost

Loading

ZeroIntensity commented Nov 12, 2024 •

edited

Loading