gh-101892: Fix SystemError when a callable iterator call exhausts the iterator#101896

workingpayload · 2023-02-14T09:04:44Z

Updated Cython/objects/iterobject.c Line 217

Issue: Callable iterator can SystemError if call exhausts the iterator #101892

Updated Cython/objects/iterobject.c Line 217

Eclips4 · 2023-02-14T09:14:30Z

It's good to coverage this case in tests.

arhadthedev · 2023-02-14T19:21:53Z

Added a test copied from the issue with readability changes.

OTheDev · 2023-02-15T04:59:49Z

Objects/iterobject.c


 result=_PyObject_CallNoArgs(it->it_callable);
-if (result!=NULL){
+if (result!=NULL&&it->it_callable!=NULL){


Sorry, but this won't fix it. Notice the first if statement in the function. If it->it_callable == NULL is true, NULL is returned. So it->it_callable != NULL is already guaranteed at this point.

What if we check it->it_sentinel != NULL ?

OTheDev

It looks like NULL is being passed to PyObject_RichCompareBool() through it->it_sentinel when it expects an object?

If we add the check

if (result!=NULL&&it->it_sentinel!=NULL){

and get rid of that Py_DECREF(result) and place a new

Py_XDECREF(result);

just before return NULL;

Then the code in the issue will raise a StopIteration (no longer SystemError) and tests pass.

I'm not entirely sure if this is correct, TBH, but this is my guess.

OTheDev · 2023-02-15T06:03:48Z

Lib/test/test_iter.py

+returnNO_MORE
+spam.need_reentrance=True
+spam.iterator=iter(spam, NO_MORE)
+next(spam.iterator)


What's the expected behaviour here? StopIteration?

Thank you for noticing the oversight! Fixed.

OTheDev · 2023-02-15T07:12:56Z

@workingpayload What do you think? Worth a try?

workingpayload · 2023-02-15T07:15:41Z

@workingpayload What do you think? Worth a try?

Yeah! Why not?

arhadthedev · 2023-02-15T08:04:35Z

Lib/test/test_iter.py

+defspam():
+ifspam.need_reentrance:
+spam.need_reentrance=False
+list(spam.iterator) # Implicitly call ourselves


This evening (after getting access to my dev machine) I'll check whether the inner list() is required to throw StopIteration to trigger the bug. If no, I will simplify the test:
Suggested change
list(spam.iterator) # Implicitly call ourselves
try:
list(spam.iterator) # Implicitly call ourselves.
exceptStopIteration:
pass

I got it wrong. list() cannot throw an exception because it returns a ready-to-use list (possibly empty) processing StopIteration by itself.

furkanonder · 2023-02-15T22:23:46Z

LGTM. By the way PR title can be more meaningful. Ex: Fix SystemError when a callable iterator call exhausts the iterator.

Lib/test/test_iter.py

arhadthedev · 2023-03-04T11:34:23Z

@kumaraditya303 Thank you, addressed.

miss-islington · 2023-03-04T14:21:33Z

Thanks @workingpayload for the PR, and @kumaraditya303 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington · 2023-03-04T14:21:59Z

Sorry, @workingpayload and @kumaraditya303, I could not cleanly backport this to 3.10 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 705487c6557c3d8866622b4d32528bf7fc2e4204 3.10

bedevere-bot · 2023-03-04T14:22:01Z

GH-102418 is a backport of this pull request to the 3.11 branch.

…usts the iterator (pythonGH-101896) (cherry picked from commit 705487c) Co-authored-by: Raj <51259329+workingpayload@users.noreply.github.com> Co-authored-by: Oleg Iarygin <oleg@arhadthedev.net>

…he iterator (GH-101896) (cherry picked from commit 705487c) Co-authored-by: Raj <51259329+workingpayload@users.noreply.github.com> Co-authored-by: Oleg Iarygin <oleg@arhadthedev.net>

…usts the iterator (python#101896) Co-authored-by: Oleg Iarygin <oleg@arhadthedev.net> (cherry picked from commit 705487c)

bedevere-bot · 2023-03-04T14:49:37Z

GH-102422 is a backport of this pull request to the 3.10 branch.

…austs the iterator (GH-101896) (#102422) gh-101892: Fix `SystemError` when a callable iterator call exhausts the iterator (#101896) Co-authored-by: Oleg Iarygin <oleg@arhadthedev.net> (cherry picked from commit 705487c) Co-authored-by: Raj <51259329+workingpayload@users.noreply.github.com>

OTheDev · 2023-03-04T15:40:20Z

@arhadthedev Thanks for the continued work on this!

…usts the iterator (python#101896) Co-authored-by: Oleg Iarygin <oleg@arhadthedev.net>

* main: (21 commits) pythongh-102192: Replace PyErr_Fetch/Restore etc by more efficient alternatives in sub interpreters module (python#102472) pythongh-95672: Fix versionadded indentation of get_pagesize in test.rst (pythongh-102455) pythongh-102416: Do not memoize incorrectly loop rules in the parser (python#102467) pythonGH-101362: Optimise PurePath(PurePath(...)) (pythonGH-101667) pythonGH-101362: Check pathlib.Path flavour compatibility at import time (pythonGH-101664) pythonGH-101362: Call join() only when >1 argument supplied to pathlib.PurePath() (python#101665) pythongh-102444: Fix minor bugs in `test_typing` highlighted by pyflakes (python#102445) pythonGH-102341: Improve the test function for pow (python#102342) Fix unused classes in a typing test (pythonGH-102437) pythongh-101979: argparse: fix a bug where parentheses in metavar argument of add_argument() were dropped (python#102318) pythongh-102356: Add thrashcan macros to filter object dealloc (python#102426) Move around example in to_bytes() to avoid confusion (python#101595) pythonGH-97546: fix flaky asyncio `test_wait_for_race_condition` test (python#102421) pythongh-96821: Add config option `--with-strict-overflow` (python#96823) pythongh-101992: update pstlib module documentation (python#102133) pythongh-63301: Set exit code when tabnanny CLI exits on error (python#7699) pythongh-101863: Fix wrong comments in EUC-KR codec (pythongh-102417) pythongh-102302 Micro-optimize `inspect.Parameter.__hash__` (python#102303) pythongh-102179: Fix `os.dup2` error reporting for negative fds (python#102180) pythongh-101892: Fix `SystemError` when a callable iterator call exhausts the iterator (python#101896) ...

* main: (37 commits) pythongh-102192: Replace PyErr_Fetch/Restore etc by more efficient alternatives in sub interpreters module (python#102472) pythongh-95672: Fix versionadded indentation of get_pagesize in test.rst (pythongh-102455) pythongh-102416: Do not memoize incorrectly loop rules in the parser (python#102467) pythonGH-101362: Optimise PurePath(PurePath(...)) (pythonGH-101667) pythonGH-101362: Check pathlib.Path flavour compatibility at import time (pythonGH-101664) pythonGH-101362: Call join() only when >1 argument supplied to pathlib.PurePath() (python#101665) pythongh-102444: Fix minor bugs in `test_typing` highlighted by pyflakes (python#102445) pythonGH-102341: Improve the test function for pow (python#102342) Fix unused classes in a typing test (pythonGH-102437) pythongh-101979: argparse: fix a bug where parentheses in metavar argument of add_argument() were dropped (python#102318) pythongh-102356: Add thrashcan macros to filter object dealloc (python#102426) Move around example in to_bytes() to avoid confusion (python#101595) pythonGH-97546: fix flaky asyncio `test_wait_for_race_condition` test (python#102421) pythongh-96821: Add config option `--with-strict-overflow` (python#96823) pythongh-101992: update pstlib module documentation (python#102133) pythongh-63301: Set exit code when tabnanny CLI exits on error (python#7699) pythongh-101863: Fix wrong comments in EUC-KR codec (pythongh-102417) pythongh-102302 Micro-optimize `inspect.Parameter.__hash__` (python#102303) pythongh-102179: Fix `os.dup2` error reporting for negative fds (python#102180) pythongh-101892: Fix `SystemError` when a callable iterator call exhausts the iterator (python#101896) ...

pythongh-101892: Updated Cython/objects/iterobject.c Line 217
de17a82
Updated Cython/objects/iterobject.c Line 217

bedevere-bot added the awaiting review label Feb 14, 2023

bedevere-bot mentioned this pull request Feb 14, 2023
Callable iterator can SystemError if call exhausts the iterator #101892
Closed

📜🤖 Added by blurb_it.
3fba831

arhadthedev added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Feb 14, 2023

Clarify the news entry
bee2af7

workingpayloadand others added 2 commits February 14, 2023 16:17

Merge branch 'main' into workingpayload/issue101892
25f4c06

Add a test
0c631e3

Amend wording
506fb6a

OTheDev reviewed Feb 15, 2023
View reviewed changes

Rep
78168bf

OTheDev reviewed Feb 15, 2023
View reviewed changes

Address the review
188e388

pythongh-101892: Updated Cython/objects/iterobject.c Line 217
0f9cea1

workingpayloadand others added 3 commits February 15, 2023 13:14

Merge branch 'main' into workingpayload/issue101892
6abb09a

Update test_iter.py
c8b6bad

Remove an unnecessary abstraction of reentrance
6931c20

arhadthedev reviewed Feb 15, 2023
View reviewed changes

arhadthedev added 3 commits February 15, 2023 22:47

Clarify comments once more
36b043c

Update 2023-02-14-09-08-48.gh-issue-101892.FMos8l.rst
340f668

Fix issues reported by patchcheck
fe1502d

furkanonder approved these changes Feb 15, 2023
View reviewed changes

bedevere-bot added awaiting core review and removed awaiting review labels Feb 15, 2023

AlexWaygood requested review from kumaraditya303 and removed request for OTheDev March 4, 2023 10:11

AlexWaygood added awaiting change review and removed awaiting changes labels Mar 4, 2023

kumaraditya303 reviewed Mar 4, 2023
View reviewed changes

Lib/test/test_iter.py Outdated Show resolvedHide resolved
Lib/test/test_iter.py Outdated Show resolvedHide resolved

Address the review
1ee2293

kumaraditya303 approved these changes Mar 4, 2023
View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Mar 4, 2023

kumaraditya303 added needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes labels Mar 4, 2023

kumaraditya303 merged commit 705487c into python:mainMar 4, 2023

bedevere-bot removed the awaiting merge label Mar 4, 2023

miss-islington assigned kumaraditya303 Mar 4, 2023

bedevere-bot removed the needs backport to 3.11 only security fixes label Mar 4, 2023

bedevere-bot removed the needs backport to 3.10 only security fixes label Mar 4, 2023

workingpayload deleted the workingpayload/issue101892 branch March 5, 2023 07:42

hugovk pushed a commit to hugovk/cpython that referenced this pull request Mar 6, 2023
pythongh-101892: FixSystemErrorwhen a callable iterator call exha…
bf01a62
…usts the iterator (python#101896) Co-authored-by: Oleg Iarygin <oleg@arhadthedev.net>

-list(spam.iterator) # Implicitly call ourselves
+try:
+list(spam.iterator) # Implicitly call ourselves.
+exceptStopIteration:
+pass

Uh oh!

gh-101892: Fix SystemError when a callable iterator call exhausts the iterator#101896

gh-101892: Fix SystemError when a callable iterator call exhausts the iterator #101896

Uh oh!

Conversation

workingpayload commented Feb 14, 2023• edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Eclips4 commented Feb 14, 2023

Uh oh!

arhadthedev commented Feb 14, 2023

Uh oh!

OTheDevFeb 15, 2023

Choose a reason for hiding this comment

Uh oh!

workingpayloadFeb 15, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OTheDev left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OTheDevFeb 15, 2023

Choose a reason for hiding this comment

Uh oh!

arhadthedevFeb 15, 2023

Choose a reason for hiding this comment

Uh oh!

OTheDev commented Feb 15, 2023

Uh oh!

workingpayload commented Feb 15, 2023

Uh oh!

arhadthedevFeb 15, 2023

Choose a reason for hiding this comment

Uh oh!

arhadthedevFeb 15, 2023

Choose a reason for hiding this comment

Uh oh!

furkanonder commented Feb 15, 2023

Uh oh!

Uh oh!

Uh oh!

arhadthedev commented Mar 4, 2023

Uh oh!

miss-islington commented Mar 4, 2023

Uh oh!

miss-islington commented Mar 4, 2023

Uh oh!

bedevere-bot commented Mar 4, 2023

Uh oh!

bedevere-bot commented Mar 4, 2023

Uh oh!

OTheDev commented Mar 4, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

workingpayload commented Feb 14, 2023•
edited by bedevere-bot
Loading

workingpayloadFeb 15, 2023•
edited
Loading

OTheDev left a comment •
edited
Loading