classification
Title: Possible fatal errors due to _PyEval_SetAsyncGen{Finalizer,Firstiter}()
Type: crash Stage: resolved
Components: Interpreter Core Versions: Python 3.9, Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: ZackerySpytz, lukasz.langa, serhiy.storchaka, steve.dower
Priority: deferred blocker Keywords: newcomer friendly, patch

Created on 2019-10-08 14:06 by ZackerySpytz, last changed 2020-03-26 12:12 by steve.dower. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 16657 merged ZackerySpytz, 2019-10-08 14:08
PR 18658 merged serhiy.storchaka, 2020-02-25 19:02
Messages (15)
msg354210 - (view) Author: Zackery Spytz (ZackerySpytz) * (Python triager) Date: 2019-10-08 14:06
_PyEval_SetAsyncGenFinalizer() and _PyEval_SetAsyncGenFirstiter() don't include proper error handling for their PySys_Audit() calls. This could lead to leaked exceptions and fatal errors.
msg354220 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-10-08 15:47
You're right, they need either your patch or PyErr_WriteUnraisable(NULL) before returning.

Łukasz - this needs a fix in 3.8, but we don't have to necessarily change the (internal, but exposed) ABI. For 3.9, we'll fix it properly, but for 3.8 I'll let you make the call whether we can also add a return value to these functions.

-PyAPI_FUNC(void) _PyEval_SetAsyncGenFirstiter(PyObject *);
+PyAPI_FUNC(int) _PyEval_SetAsyncGenFirstiter(PyObject *);
 PyAPI_FUNC(PyObject *) _PyEval_GetAsyncGenFirstiter(void);
-PyAPI_FUNC(void) _PyEval_SetAsyncGenFinalizer(PyObject *);
+PyAPI_FUNC(int) _PyEval_SetAsyncGenFinalizer(PyObject *);
msg354602 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2019-10-13 21:19
Unfortunately at this point we will have to leave the ABI as is. We are in fact promising to lock it by Beta 3 so quite a long time ago.
msg354637 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-10-14 15:41
In that case, the fix for 3.8 should be to call PyErr_WriteUnraisable(NULL) when an error occurs. This will report it through the new unraisable hook, and also prevent a fatal error if the exception escapes.
msg354641 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-10-14 16:35
But I'm okay to defer it - this is still going to be obscure functionality for the time being, and it's possible to avoid the issue.
msg354868 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-10-18 07:58
Why this ABI is exported at all? These functions are only used in the sys module, why they are defined in ceval.c instead of be static functions in sysmodule.c?
msg354910 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2019-10-18 16:33
> Why this ABI is exported at all?

Great question for (probably) Yury, but it is exported and so we're stuck with it for 3.8 at least :(
msg358102 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2019-12-09 14:26
Note: this is going to miss Python 3.8.1 as I'm releasing 3.8.1rc1 right now.  Please address this before 3.8.2 (due in February).
msg362638 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2020-02-25 12:13
Sadly, this missed the train for 3.8.2. Steve, what's the status of the open PR for this?
msg362652 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2020-02-25 17:39
I thought we weren't going to take the fix for 3.8 anyway? (Or did we make a smaller one and commit it directly, apparently without linking it to the issue correctly?)

The status is the PR needs conflicts resolved, and people need to stop adding to my infinite list of GitHub notifications and ping me on here instead :)
msg362659 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-02-25 19:06
PR 18658 is a backport to 3.8 which preserves binary compatibility.
msg363135 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2020-03-02 07:58
New changeset 87a4cd5fbebdd0e6166b421d2c3706bc2f2e5a11 by Serhiy Storchaka in branch '3.8':
bpo-38410: Properly handle PySys_Audit() failures (GH-18658)
https://github.com/python/cpython/commit/87a4cd5fbebdd0e6166b421d2c3706bc2f2e5a11
msg365057 - (view) Author: Zackery Spytz (ZackerySpytz) * (Python triager) Date: 2020-03-26 09:51
I had pinged Steve Dower on the PR, but I didn't realize they weren't notified. The merge conflicts have been fixed.
msg365062 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2020-03-26 12:11
Sorry for missing the pings! My GitHub notifications are a bit of a black hole. I'll merge it now.
msg365063 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2020-03-26 12:11
New changeset 79ceccd1ec6ef7e487da2916f32c6f0d1477bd3d by Zackery Spytz in branch 'master':
bpo-38410: Properly handle PySys_Audit() failures (GH-16657)
https://github.com/python/cpython/commit/79ceccd1ec6ef7e487da2916f32c6f0d1477bd3d
History
Date User Action Args
2020-03-26 12:12:21steve.dowersetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2020-03-26 12:11:39steve.dowersetmessages: + msg365063
2020-03-26 12:11:00steve.dowersetmessages: + msg365062
2020-03-26 09:51:12ZackerySpytzsetmessages: + msg365057
2020-03-02 07:58:51serhiy.storchakasetmessages: + msg363135
2020-02-25 19:06:21serhiy.storchakasetmessages: + msg362659
2020-02-25 19:02:28serhiy.storchakasetpull_requests: + pull_request18019
2020-02-25 17:39:08steve.dowersetmessages: + msg362652
2020-02-25 12:13:01lukasz.langasetmessages: + msg362638
2019-12-09 14:26:48lukasz.langasetmessages: + msg358102
2019-10-18 16:33:04steve.dowersetmessages: + msg354910
2019-10-18 07:58:26serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg354868
2019-10-14 16:35:03steve.dowersetpriority: release blocker -> deferred blocker

messages: + msg354641
2019-10-14 15:41:18steve.dowersetmessages: + msg354637
versions: + Python 3.8
2019-10-13 21:19:17lukasz.langasetmessages: + msg354602
versions: - Python 3.8
2019-10-08 15:47:53steve.dowersetpriority: normal -> release blocker

nosy: + lukasz.langa
messages: + msg354220

keywords: + newcomer friendly
2019-10-08 14:38:52xtreaksetnosy: + steve.dower
2019-10-08 14:08:56ZackerySpytzsetkeywords: + patch
stage: patch review
pull_requests: + pull_request16240
2019-10-08 14:06:14ZackerySpytzcreate