Skip to content

gh-111375: Use NULL in the exception stack to indicate an exception was handled #113302

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.

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

Merged

Conversation

pR0Ps
Copy link
Contributor

@pR0Ps pR0Ps commented Dec 20, 2023

Previously, both NULL and Py_None would be used interchangeably to indicate that an exception is no longer being handled. By ensuring that only NULL is used, this opens up the possibility to use Py_None to indicate a cleared exception. The difference here would be that clearing would indicate that no exception is currently being handled vs. handling would indicate that the next exception in the stack is currently being handled.

This functionality will be used to patch up some edge cases in how the exception context interacts with exceptions thrown into coroutines. See #111676 (comment) for context.

This is implemented in this commit by changing code that could add Py_None to the exception stack to indicate that an exception is no longer being handled to add NULL instead. An assert was also added to ensure that Py_None is no longer added to the exception stack.

Possible reviewers based on git blame: @iritkatriel , @markshannon

@pR0Ps pR0Ps changed the title gh-111676: Use NULL in the exception stack to indicate an exception was handled gh-111375: Use NULL in the exception stack to indicate an exception was handled Dec 20, 2023
Previously, both `NULL` and `Py_None` would be used interchangeably to
indicate that an exception is no longer being handled. By ensuring that
only `NULL` is used, this opens up the possibility to use `Py_None` to
indicate a cleared exception. The difference here would be that clearing
would indicate that no exception is currently being handled vs. handling
would indicate that the next exception in the stack is currently being
handled.

This functionality will be used to patch up some edge cases in how the
exception context interacts with exceptions thrown into coroutines.

This is implemented in this commit by changing code that could add
`Py_None` to the exception stack to indicate that an exception is no
longer being handled to add `NULL` instead. An assert was also added to
ensure that `Py_None` is no longer added to the exception stack.

See pythongh-111676 for context.
@iritkatriel iritkatriel added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 20, 2023
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @iritkatriel for commit 4ac01c0 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Dec 20, 2023
@iritkatriel
Copy link
Member

I checked a couple of the failed builedbots and the failures seem unrelated (and also appear in the previous runs). But we should check all of them to be sure there's nothing new before merging. Some buildbots are still running.

@iritkatriel iritkatriel merged commit a2dd0e7 into python:main Dec 21, 2023
@pR0Ps pR0Ps deleted the bugfix/consistent-exception-stack-values branch December 21, 2023 14:28
ryan-duve pushed a commit to ryan-duve/cpython that referenced this pull request Dec 26, 2023
… to indicate that an exception was handled (python#113302)
kulikjak pushed a commit to kulikjak/cpython that referenced this pull request Jan 22, 2024
… to indicate that an exception was handled (python#113302)
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
… to indicate that an exception was handled (python#113302)
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
… to indicate that an exception was handled (python#113302)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants