From 52978652a435bc68f5f46b9ba2a537330a52dbab Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Wed, 1 May 2024 22:02:33 +0900 Subject: [PATCH 1/3] gh-118473: Fix set_asyncgen_hooks not to allow partial setup --- Include/internal/pycore_ceval.h | 4 +-- Lib/test/test_sys.py | 16 +++++++++++- ...-05-01-22-43-54.gh-issue-118473.QIvq9R.rst | 1 + Python/ceval.c | 14 ++-------- Python/sysmodule.c | 26 ++++++++++++++----- 5 files changed, 39 insertions(+), 22 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2024-05-01-22-43-54.gh-issue-118473.QIvq9R.rst diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h index cfb88c3f4c8e15..fa21ca21d503d7 100644 --- a/Include/internal/pycore_ceval.h +++ b/Include/internal/pycore_ceval.h @@ -71,8 +71,8 @@ extern PyObject* _PyEval_GetAsyncGenFirstiter(void); extern PyObject* _PyEval_GetAsyncGenFinalizer(void); // Used by sys.set_asyncgen_hooks() -extern int _PyEval_SetAsyncGenFirstiter(PyObject *); -extern int _PyEval_SetAsyncGenFinalizer(PyObject *); +extern void _PyEval_SetAsyncGenFirstiter(PyObject *); +extern void _PyEval_SetAsyncGenFinalizer(PyObject *); // Used by sys.get_coroutine_origin_tracking_depth() // and sys.set_coroutine_origin_tracking_depth() diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index 14ec51eb757e00..73b4bbbebb972c 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -1779,6 +1779,21 @@ def test_asyncgen_hooks(self): self.assertIsNone(old.finalizer) firstiter = lambda *a: None + finalizer = lambda *a: None + + with self.assertRaises(TypeError): + sys.set_asyncgen_hooks(firstiter=firstiter, finalizer="invalid") + cur = sys.get_asyncgen_hooks() + self.assertIsNone(cur.firstiter) + self.assertIsNone(cur.finalizer) + + # gh-118473 + with self.assertRaises(TypeError): + sys.set_asyncgen_hooks(firstiter="invalid", finalizer=finalizer) + cur = sys.get_asyncgen_hooks() + self.assertIsNone(cur.firstiter) + self.assertIsNone(cur.finalizer) + sys.set_asyncgen_hooks(firstiter=firstiter) hooks = sys.get_asyncgen_hooks() self.assertIs(hooks.firstiter, firstiter) @@ -1786,7 +1801,6 @@ def test_asyncgen_hooks(self): self.assertIs(hooks.finalizer, None) self.assertIs(hooks[1], None) - finalizer = lambda *a: None sys.set_asyncgen_hooks(finalizer=finalizer) hooks = sys.get_asyncgen_hooks() self.assertIs(hooks.firstiter, firstiter) diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-05-01-22-43-54.gh-issue-118473.QIvq9R.rst b/Misc/NEWS.d/next/Core and Builtins/2024-05-01-22-43-54.gh-issue-118473.QIvq9R.rst new file mode 100644 index 00000000000000..91430925938dc6 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-05-01-22-43-54.gh-issue-118473.QIvq9R.rst @@ -0,0 +1 @@ +Fix set_asyncgen_hooks not to be partially set when raising TypeError diff --git a/Python/ceval.c b/Python/ceval.c index 59498bc826e941..937a4de771d204 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -2366,17 +2366,12 @@ _PyEval_GetCoroutineOriginTrackingDepth(void) return tstate->coroutine_origin_tracking_depth; } -int +void _PyEval_SetAsyncGenFirstiter(PyObject *firstiter) { PyThreadState *tstate = _PyThreadState_GET(); - if (_PySys_Audit(tstate, "sys.set_asyncgen_hook_firstiter", NULL) < 0) { - return -1; - } - Py_XSETREF(tstate->async_gen_firstiter, Py_XNewRef(firstiter)); - return 0; } PyObject * @@ -2386,17 +2381,12 @@ _PyEval_GetAsyncGenFirstiter(void) return tstate->async_gen_firstiter; } -int +void _PyEval_SetAsyncGenFinalizer(PyObject *finalizer) { PyThreadState *tstate = _PyThreadState_GET(); - if (_PySys_Audit(tstate, "sys.set_asyncgen_hook_finalizer", NULL) < 0) { - return -1; - } - Py_XSETREF(tstate->async_gen_finalizer, Py_XNewRef(finalizer)); - return 0; } PyObject * diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 726051521cf574..6f545205e2a665 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -1391,6 +1391,7 @@ static PyObject * sys_set_asyncgen_hooks(PyObject *self, PyObject *args, PyObject *kw) { static char *keywords[] = {"firstiter", "finalizer", NULL}; + PyThreadState *tstate = NULL; PyObject *firstiter = NULL; PyObject *finalizer = NULL; @@ -1400,6 +1401,8 @@ sys_set_asyncgen_hooks(PyObject *self, PyObject *args, PyObject *kw) return NULL; } + tstate = _PyThreadState_GET(); + if (finalizer && finalizer != Py_None) { if (!PyCallable_Check(finalizer)) { PyErr_Format(PyExc_TypeError, @@ -1407,27 +1410,36 @@ sys_set_asyncgen_hooks(PyObject *self, PyObject *args, PyObject *kw) Py_TYPE(finalizer)->tp_name); return NULL; } - if (_PyEval_SetAsyncGenFinalizer(finalizer) < 0) { + if (_PySys_Audit(tstate, "sys.set_asyncgen_hook_finalizer", NULL) < 0) { return NULL; } } - else if (finalizer == Py_None && _PyEval_SetAsyncGenFinalizer(NULL) < 0) { - return NULL; - } if (firstiter && firstiter != Py_None) { if (!PyCallable_Check(firstiter)) { PyErr_Format(PyExc_TypeError, "callable firstiter expected, got %.50s", Py_TYPE(firstiter)->tp_name); + return NULL; } - if (_PyEval_SetAsyncGenFirstiter(firstiter) < 0) { + if (_PySys_Audit(tstate, "sys.set_asyncgen_hook_firstiter", NULL) < 0) { return NULL; } } - else if (firstiter == Py_None && _PyEval_SetAsyncGenFirstiter(NULL) < 0) { - return NULL; + + if (finalizer && finalizer != Py_None) { + _PyEval_SetAsyncGenFinalizer(finalizer); + } + else if (finalizer == Py_None) { + _PyEval_SetAsyncGenFinalizer(NULL); + } + + if (firstiter && firstiter != Py_None) { + _PyEval_SetAsyncGenFirstiter(firstiter); + } + else if (firstiter == Py_None) { + _PyEval_SetAsyncGenFirstiter(NULL); } Py_RETURN_NONE; From f2fa7fb72910d590685daf8d8afb2506afde3fea Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 2 May 2024 11:42:19 +0900 Subject: [PATCH 2/3] Apply review --- Include/internal/pycore_ceval.h | 4 +-- ...-05-01-22-43-54.gh-issue-118473.QIvq9R.rst | 2 +- Python/ceval.c | 14 ++++++-- Python/sysmodule.c | 34 ++++++++----------- 4 files changed, 29 insertions(+), 25 deletions(-) diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h index fa21ca21d503d7..cfb88c3f4c8e15 100644 --- a/Include/internal/pycore_ceval.h +++ b/Include/internal/pycore_ceval.h @@ -71,8 +71,8 @@ extern PyObject* _PyEval_GetAsyncGenFirstiter(void); extern PyObject* _PyEval_GetAsyncGenFinalizer(void); // Used by sys.set_asyncgen_hooks() -extern void _PyEval_SetAsyncGenFirstiter(PyObject *); -extern void _PyEval_SetAsyncGenFinalizer(PyObject *); +extern int _PyEval_SetAsyncGenFirstiter(PyObject *); +extern int _PyEval_SetAsyncGenFinalizer(PyObject *); // Used by sys.get_coroutine_origin_tracking_depth() // and sys.set_coroutine_origin_tracking_depth() diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-05-01-22-43-54.gh-issue-118473.QIvq9R.rst b/Misc/NEWS.d/next/Core and Builtins/2024-05-01-22-43-54.gh-issue-118473.QIvq9R.rst index 91430925938dc6..9d65e3c403ff88 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2024-05-01-22-43-54.gh-issue-118473.QIvq9R.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2024-05-01-22-43-54.gh-issue-118473.QIvq9R.rst @@ -1 +1 @@ -Fix set_asyncgen_hooks not to be partially set when raising TypeError +Fix :func:`sys.set_asyncgen_hooks` not to be partially set when raising :exc:`TypeError`. diff --git a/Python/ceval.c b/Python/ceval.c index 937a4de771d204..59498bc826e941 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -2366,12 +2366,17 @@ _PyEval_GetCoroutineOriginTrackingDepth(void) return tstate->coroutine_origin_tracking_depth; } -void +int _PyEval_SetAsyncGenFirstiter(PyObject *firstiter) { PyThreadState *tstate = _PyThreadState_GET(); + if (_PySys_Audit(tstate, "sys.set_asyncgen_hook_firstiter", NULL) < 0) { + return -1; + } + Py_XSETREF(tstate->async_gen_firstiter, Py_XNewRef(firstiter)); + return 0; } PyObject * @@ -2381,12 +2386,17 @@ _PyEval_GetAsyncGenFirstiter(void) return tstate->async_gen_firstiter; } -void +int _PyEval_SetAsyncGenFinalizer(PyObject *finalizer) { PyThreadState *tstate = _PyThreadState_GET(); + if (_PySys_Audit(tstate, "sys.set_asyncgen_hook_finalizer", NULL) < 0) { + return -1; + } + Py_XSETREF(tstate->async_gen_finalizer, Py_XNewRef(finalizer)); + return 0; } PyObject * diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 6f545205e2a665..9142a78c259af2 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -1391,7 +1391,6 @@ static PyObject * sys_set_asyncgen_hooks(PyObject *self, PyObject *args, PyObject *kw) { static char *keywords[] = {"firstiter", "finalizer", NULL}; - PyThreadState *tstate = NULL; PyObject *firstiter = NULL; PyObject *finalizer = NULL; @@ -1401,7 +1400,7 @@ sys_set_asyncgen_hooks(PyObject *self, PyObject *args, PyObject *kw) return NULL; } - tstate = _PyThreadState_GET(); + PyObject *cur_finalizer = _PyEval_GetAsyncGenFinalizer(); if (finalizer && finalizer != Py_None) { if (!PyCallable_Check(finalizer)) { @@ -1410,39 +1409,34 @@ sys_set_asyncgen_hooks(PyObject *self, PyObject *args, PyObject *kw) Py_TYPE(finalizer)->tp_name); return NULL; } - if (_PySys_Audit(tstate, "sys.set_asyncgen_hook_finalizer", NULL) < 0) { + if (_PyEval_SetAsyncGenFinalizer(finalizer) < 0) { return NULL; } } + else if (finalizer == Py_None && _PyEval_SetAsyncGenFinalizer(NULL) < 0) { + return NULL; + } if (firstiter && firstiter != Py_None) { if (!PyCallable_Check(firstiter)) { PyErr_Format(PyExc_TypeError, "callable firstiter expected, got %.50s", Py_TYPE(firstiter)->tp_name); - - return NULL; + goto error; } - if (_PySys_Audit(tstate, "sys.set_asyncgen_hook_firstiter", NULL) < 0) { - return NULL; + if (_PyEval_SetAsyncGenFirstiter(firstiter) < 0) { + goto error; } } - - if (finalizer && finalizer != Py_None) { - _PyEval_SetAsyncGenFinalizer(finalizer); - } - else if (finalizer == Py_None) { - _PyEval_SetAsyncGenFinalizer(NULL); - } - - if (firstiter && firstiter != Py_None) { - _PyEval_SetAsyncGenFirstiter(firstiter); - } - else if (firstiter == Py_None) { - _PyEval_SetAsyncGenFirstiter(NULL); + else if (firstiter == Py_None && _PyEval_SetAsyncGenFirstiter(NULL) < 0) { + goto error; } Py_RETURN_NONE; + +error: + _PyEval_SetAsyncGenFinalizer(cur_finalizer); + return NULL; } PyDoc_STRVAR(set_asyncgen_hooks_doc, From 3e603054a592844c9812318b6589ef6944d9825c Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 2 May 2024 11:44:08 +0900 Subject: [PATCH 3/3] Separate callable check step --- Python/sysmodule.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 9142a78c259af2..5ca5870981d431 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -1400,8 +1400,6 @@ sys_set_asyncgen_hooks(PyObject *self, PyObject *args, PyObject *kw) return NULL; } - PyObject *cur_finalizer = _PyEval_GetAsyncGenFinalizer(); - if (finalizer && finalizer != Py_None) { if (!PyCallable_Check(finalizer)) { PyErr_Format(PyExc_TypeError, @@ -1409,6 +1407,20 @@ sys_set_asyncgen_hooks(PyObject *self, PyObject *args, PyObject *kw) Py_TYPE(finalizer)->tp_name); return NULL; } + } + + if (firstiter && firstiter != Py_None) { + if (!PyCallable_Check(firstiter)) { + PyErr_Format(PyExc_TypeError, + "callable firstiter expected, got %.50s", + Py_TYPE(firstiter)->tp_name); + return NULL; + } + } + + PyObject *cur_finalizer = _PyEval_GetAsyncGenFinalizer(); + + if (finalizer && finalizer != Py_None) { if (_PyEval_SetAsyncGenFinalizer(finalizer) < 0) { return NULL; } @@ -1418,12 +1430,6 @@ sys_set_asyncgen_hooks(PyObject *self, PyObject *args, PyObject *kw) } if (firstiter && firstiter != Py_None) { - if (!PyCallable_Check(firstiter)) { - PyErr_Format(PyExc_TypeError, - "callable firstiter expected, got %.50s", - Py_TYPE(firstiter)->tp_name); - goto error; - } if (_PyEval_SetAsyncGenFirstiter(firstiter) < 0) { goto error; }