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

@@ -214,7 +214,7 @@ calliter_iternext(calliterobject *it)
    }

    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

+            return NO_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

+        def spam():
+            if spam.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.

except StopIteration:

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

workingpayload and 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

workingpayload and 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 resolved Hide resolved

Lib/test/test_iter.py Outdated Show resolved Hide 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:main Mar 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: Fix SystemError when a callable iterator call exha…

bf01a62

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

workingpayload commented Feb 14, 2023 •

edited by bedevere-bot

Loading

Eclips4 commented Feb 14, 2023

arhadthedev commented Feb 14, 2023

OTheDev Feb 15, 2023

workingpayload Feb 15, 2023 •

edited

Loading

OTheDev left a comment •

edited

Loading

OTheDev Feb 15, 2023

arhadthedev Feb 15, 2023

OTheDev commented Feb 15, 2023

workingpayload commented Feb 15, 2023

arhadthedev Feb 15, 2023

arhadthedev Feb 15, 2023

furkanonder commented Feb 15, 2023

arhadthedev commented Mar 4, 2023

miss-islington commented Mar 4, 2023

miss-islington commented Mar 4, 2023

bedevere-bot commented Mar 4, 2023

bedevere-bot commented Mar 4, 2023

OTheDev commented Mar 4, 2023

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

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

Conversation

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

Eclips4 commented Feb 14, 2023

arhadthedev commented Feb 14, 2023

OTheDev Feb 15, 2023

Choose a reason for hiding this comment

workingpayload Feb 15, 2023 • edited Loading

Choose a reason for hiding this comment

OTheDev left a comment • edited Loading

Choose a reason for hiding this comment

OTheDev Feb 15, 2023

Choose a reason for hiding this comment

arhadthedev Feb 15, 2023

Choose a reason for hiding this comment

OTheDev commented Feb 15, 2023

workingpayload commented Feb 15, 2023

arhadthedev Feb 15, 2023

Choose a reason for hiding this comment

arhadthedev Feb 15, 2023

Choose a reason for hiding this comment

furkanonder commented Feb 15, 2023

arhadthedev commented Mar 4, 2023

miss-islington commented Mar 4, 2023

miss-islington commented Mar 4, 2023

bedevere-bot commented Mar 4, 2023

bedevere-bot commented Mar 4, 2023

OTheDev commented Mar 4, 2023

workingpayload commented Feb 14, 2023 •

edited by bedevere-bot

Loading

workingpayload Feb 15, 2023 •

edited

Loading

OTheDev left a comment •

edited

Loading