8000 `STOP_ITERATION` monitoring event is not triggered by unspecialized bytecode · Issue #116090 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

STOP_ITERATION monitoring event is not triggered by unspecialized bytecode #116090

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
Closed
swtaarrs opened this issue Feb 29, 2024 · 6 comments
Closed

STOP_ITERATION monitoring event is not triggered by unspecialized bytecode #116090

swtaarrs opened this issue Feb 29, 2024 · 6 comments
Assignees
@markshannon
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@swtaarrs
Copy link
Member
swtaarrs commented Feb 29, 2024 •
edited by bedevere-app bot
Loading

Bug report

Bug description:

Repro: Check out this commit, build, and run python -mtest test_monitoring -mtest_implicit_stop_iteration.

Context: The unspecialized implementation of FOR_ITER has some built-in opcode fusion, in that it skips the following END_FOR and POP_TOP when tp_iternext returns NULL. When the iterator is a generator, as it is in ExceptionMonitoringTest.test_implicit_stop_iteration(), that means we skip the monitor_stop_iteration() call in INSTRUMENTED_END_FOR.

test_implicit_stop_iteration() passes as written, though, because the FOR_ITER in the helper function has been specialized to FOR_ITER_GEN by the time the generator finishes, and FOR_ITER_GEN's implementation doesn't perform the same fusion as FOR_ITER.

This bug can be exposed by making this change to test_implicit_stop_iteration(), so the FOR_ITER stays unspecialized while iterating over the generator in question.

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

Linked PRs
@swtaarrs swtaarrs added type-bug An unexpected behavior, bug, or error interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Feb 29, 2024
@swtaarrs
Copy link
Member Author

It should be easy to remove the opcode fusion from FOR_ITER, which would let the END_FOR happen naturally. That could hurt performance a little, but maybe it's OK since common cases should be specialized anyway?

@gvanrossum
Copy link
Member

For @markshannon.

@gvanrossum gvanrossum mentioned this issue Feb 29, 2024
Merged
@bedevere-app bedevere-app bot mentioned this issue Jun 18, 2024
Merged
@markshannon
Copy link
Member

FTR, this is expected behavior. You either get a STOP_ITERATION event or a RAISE event with a StopIteration exception (and corresponding EXCEPTION_HANDLED event).

The reason for this slightly odd behavior is as follows:

  • Most tools aren't interested in StopIteration being raised as it isn't a "real" exception.
  • For performance reasons, we really don't want to raise StopIteration every time a generator exhausts.
  • If tools are interested in StopIteration events that would otherwise be optimized away, we need a way to provide those events without impacting performance for everyone else. That's the STOP_ITERATION event.

@markshannon
Copy link
Member

It should be easy to remove the opcode fusion from FOR_ITER, which would let the END_FOR happen naturally.

That would result in both the RAISE event and the STOP_ITERATION event for unoptimized exits from generators.

@markshannon
Copy link
Member
markshannon commented Jun 26, 2024 •
edited
Loading

In hindsight, we should probably tried to have done the following:

  • Not allow STOP_ITERATION to be disabled (for consistency with RAISE events)
  • Generate a STOP_ITERATION instead of a RAISE event with StopIteration when a generator or coroutine is exhausted.
  • Do not generate EXCEPTION_HANDLED events for those RAISE events that are converted to STOP_ITERATION events.

Maybe we could do this for 3.14

markshannon added a commit that referenced this issue Jul 26, 2024
@markshannon
GH-116090: Fix test and clarify behavior for exception events when ex…
2c42e13 
…hausting a generator. (GH-120697)
@bedevere-app bedevere-app bot mentioned this issue Jul 29, 2024
Merged
brandtbucher added a commit that referenced this issue Jul 29, 2024
@brandtbucher
GH-116090: Fire RAISE events from _FOR_ITER_TIER_TWO (GH-122413)
15d4cd0
brandtbucher added a commit to brandtbucher/cpython that referenced this issue Jul 29, 2024
@brandtbucher
pythonGH-116090: Fire RAISE events from _FOR_ITER_TIER_TWO (pythonGH-…
9bc7044 
…122413)

(cherry picked from commit 15d4cd0)
brandtbucher added a commit to brandtbucher/cpython that referenced this issue Jul 29, 2024
@brandtbucher
[3.13] pythonGH-116090: Fire RAISE events from _FOR_ITER_TIER_TWO (py…
dfeaf72 
…thonGH-122413).

(cherry picked from commit 15d4cd0)

Co-authored-by: Brandt Bucher <brandtbucher@microsoft.com>
brandtbucher added a commit to brandtbucher/cpython that referenced this issue Jul 29, 2024
@brandtbucher
[3.13] pythonGH-116090: Fire RAISE events from _FOR_ITER_TIER_TWO (py…
e9347f3 
…thonGH-122413).

(cherry picked from commit 15d4cd0)

Co-authored-by: Brandt Bucher <brandtbucher@microsoft.com>
@bedevere-app bedevere-app bot mentioned this issue Jul 29, 2024
Merged
brandtbucher added a commit that referenced this issue Jul 29, 2024
@brandtbucher
[3.13] GH-116090: Fire RAISE events from _FOR_ITER_TIER_TWO (GH-122419)
55554fd 
(cherry picked from commit 15d4cd0)
@markshannon
Copy link
Member

The current behavior might be a bit more complicated than it could be, but it works and any changes risk breaking backwards compatibility.

@markshannon markshannon closed this as not planned Won't fix, can't repro, duplicate, stale Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@markshannon markshannon

Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@swtaarrs @gvanrossum @markshannon
0