Title: Possible fatal errors due to _PyEval_SetAsyncGen{Finalizer,Firstiter}()
Type: crash Stage: patch review
Components: Interpreter Core Versions: Python 3.9, Python 3.8
Status: open Resolution:
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 2019-10-18 16:33 by steve.dower.

Pull Requests
URL Status Linked Edit
PR 16657 open ZackerySpytz, 2019-10-08 14:08
Messages (7)
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 :(
Date User Action Args
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