Skip to content
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

GH-101578: Normalize the current exception #101607

Merged
merged 13 commits into from
Feb 8, 2023

Conversation

markshannon
Copy link
Member

@markshannon markshannon commented Feb 6, 2023

This is a necessary step for implementing PEP 669, as instrumentation needs exceptions to be normalized.

Also fixes #101578.


.. c:function:: void PyErr_Restore1(PyObject *exc)

As of 3.12, this function is deprecated. Use :c:func:`PyErr_Restore1` instead.
Copy link
Member

Choose a reason for hiding this comment

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

?? this is PyErr_Restore1

Doc/c-api/exceptions.rst Outdated Show resolved Hide resolved

As of 3.12, this function is deprecated. Use :c:func:`PyErr_Restore1` instead.

Set the error indicator to ``exc``.
Copy link
Member

Choose a reason for hiding this comment

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

This looks like it's in the wrong place.

@markshannon markshannon requested a review from a team as a code owner February 7, 2023 12:43
PyAPI_FUNC(int) _PyErr_ExceptionMatches(
PyThreadState *tstate,
PyObject *exc);

void
_PyErr_SetRaisedException(PyThreadState *tstate, PyObject *exc);
Copy link
Member

Choose a reason for hiding this comment

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

does this need "extern" too?

PyObject *old_exc = tstate->current_exception;
tstate->current_exception = exc;
Py_XDECREF(old_exc);
}

void
_PyErr_Restore(PyThreadState *tstate, PyObject *type, PyObject *value,
PyObject *traceback)
{
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it's currently a requirement to pass in only normalised exceptions. This could break C extensions.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll add some extra logic. It will slow things down a bit, but safety first.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe you could make _PyErr_Restore to expectonly normalised exceptions, but normalise them in PyErr_Restore?

@markshannon markshannon closed this Feb 7, 2023
@markshannon markshannon reopened this Feb 7, 2023
Copy link
Member

@iritkatriel iritkatriel left a comment

Choose a reason for hiding this comment

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

LGTM. I'd run tests with the buildbots before merging though.

@markshannon markshannon added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 7, 2023
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @markshannon for commit 566f6e1 🤖

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

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 7, 2023
@markshannon markshannon added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 7, 2023
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @markshannon for commit 99f0880 🤖

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

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 7, 2023
@markshannon
Copy link
Member Author

The buildbot/PPC64LE RHEL8 Refleaks PR failure seems unrelated (the other refleak buildbots all pass) and that buildbot seems a bit flaky.

@markshannon markshannon merged commit feec49c into python:main Feb 8, 2023
@markshannon markshannon deleted the normalize-current-exception branch February 8, 2023 09:31
carljm added a commit to carljm/cpython that referenced this pull request Feb 9, 2023
* main: (82 commits)
  pythongh-101670: typo fix in PyImport_ExtendInittab() (python#101723)
  pythonGH-99293: Document that `Py_TPFLAGS_VALID_VERSION_TAG` shouldn't be used. (#pythonGH-101736)
  no-issue: Add Dong-hee Na as the cjkcodecs codeowner (pythongh-101731)
  pythongh-101678: Merge math_1_to_whatever() and math_1() (python#101730)
  pythongh-101678: refactor the math module to use special functions from c11 (pythonGH-101679)
  pythongh-85984: Remove legacy Lib/pty.py code. (python#92365)
  pythongh-98831: Use opcode metadata for stack_effect() (python#101704)
  pythongh-101283: Version was just released, so should be changed in 3.11.3 (pythonGH-101719)
  pythongh-101283: Fix use of unbound variable (pythonGH-101712)
  pythongh-101283: Improved fallback logic for subprocess with shell=True on Windows (pythonGH-101286)
  pythongh-101277: Port more itertools static types to heap types (python#101304)
  pythongh-98831: Modernize CALL and family (python#101508)
  pythonGH-101696: invalidate type version tag in `_PyStaticType_Dealloc` (python#101697)
  pythongh-100221: Fix creating dirs in `make sharedinstall` (pythonGH-100329)
  pythongh-101670: typo fix in PyImport_AppendInittab() (pythonGH-101672)
  pythongh-101196: Make isdir/isfile/exists faster on Windows (pythonGH-101324)
  pythongh-101614: Don't treat python3_d.dll as a Python DLL when checking extension modules for incompatibility (pythonGH-101615)
  pythongh-100933: Improve `check_element` helper in `test_xml_etree` (python#100934)
  pythonGH-101578: Normalize the current exception (pythonGH-101607)
  pythongh-47937: Note that Popen attributes are read-only (python#93070)
  ...
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.

PyErr_SetObject() behavior is strange and not as documented.
3 participants