Skip to content

gh-132775: Clean Up Cross-Interpreter Error Handling #135369

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ericsnowcurrently
Copy link
Member

@ericsnowcurrently ericsnowcurrently commented Jun 10, 2025

In this refactor we:

  • move some code around
  • make a couple of typedefs opaque
  • decouple errors from session state
  • improve tracebacks for propagated exceptions

This change helps simplify several upcoming changes.

};
if (override != NULL) {
err = *override;
*override = (_PyXI_error_override){0};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_call passes without the line in my case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

if (override == NULL) {
override = &_override;
}
_PyXI_errcode errcode = override->code;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For test_running, putting _override.code = errcode; in the _PyXI_ERR_ALREADY_RUNNING case (several lines below) works well for me.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@@ -1485,6 +1491,10 @@ _PyXI_excinfo_Apply(_PyXI_excinfo *info, PyObject *exctype)
if (tbexc == NULL) {
PyErr_Clear();
}
else {
PyErr_SetObject(exctype, tbexc);
return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tbexc seems to be leaking.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Thanks for tracking this down! I don't know how you find these things so quickly. 😄

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bisection might be effective using Windows binaries? Not so quick, but even I could be a tester to share the burden.

const char *msg = error->override != NULL
? error->override->msg
: error->uncaught.msg;
_set_xid_lookup_failure(tstate, NULL, msg, cause);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cause seems to be leaking.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@ericsnowcurrently ericsnowcurrently marked this pull request as draft June 11, 2025 17:45
@ericsnowcurrently
Copy link
Member Author

Hmm, I forgot to mark this as "draft". Sorry about that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants