From a8fe4bbd6b78517f640e25697338b9448c4675c1 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 20 Oct 2022 00:31:47 +0200 Subject: gh-98257: Make _PyEval_SetTrace() reentrant (#98258) Make sys.setprofile() and sys.settrace() functions reentrant. They can no long fail with: RuntimeError("Cannot install a trace function while another trace function is being installed"). Make _PyEval_SetTrace() and _PyEval_SetProfile() functions reentrant, rather than detecting and rejecting reentrant calls. Only delete the reference to function arguments once the new function is fully set, when a reentrant call is safe. Call also _PySys_Audit() earlier. --- Lib/test/test_sys_setprofile.py | 8 +-- Lib/test/test_sys_settrace.py | 8 +-- .../2022-10-14-12-29-05.gh-issue-98257.aMSMs2.rst | 3 ++ Python/ceval.c | 57 +++++----------------- 4 files changed, 20 insertions(+), 56 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2022-10-14-12-29-05.gh-issue-98257.aMSMs2.rst diff --git a/Lib/test/test_sys_setprofile.py b/Lib/test/test_sys_setprofile.py index 4c3053a..acae433 100644 --- a/Lib/test/test_sys_setprofile.py +++ b/Lib/test/test_sys_setprofile.py @@ -437,12 +437,8 @@ class TestEdgeCases(unittest.TestCase): sys.setprofile(bar) sys.setprofile(A()) - with support.catch_unraisable_exception() as cm: - sys.setprofile(foo) - self.assertEqual(cm.unraisable.object, A.__del__) - self.assertIsInstance(cm.unraisable.exc_value, RuntimeError) - - self.assertEqual(sys.getprofile(), foo) + sys.setprofile(foo) + self.assertEqual(sys.getprofile(), bar) def test_same_object(self): diff --git a/Lib/test/test_sys_settrace.py b/Lib/test/test_sys_settrace.py index 4f577c2..a448f80 100644 --- a/Lib/test/test_sys_settrace.py +++ b/Lib/test/test_sys_settrace.py @@ -2796,12 +2796,8 @@ class TestEdgeCases(unittest.TestCase): sys.settrace(bar) sys.settrace(A()) - with support.catch_unraisable_exception() as cm: - sys.settrace(foo) - self.assertEqual(cm.unraisable.object, A.__del__) - self.assertIsInstance(cm.unraisable.exc_value, RuntimeError) - - self.assertEqual(sys.gettrace(), foo) + sys.settrace(foo) + self.assertEqual(sys.gettrace(), bar) def test_same_object(self): diff --git a/Misc/NEWS.d/next/Library/2022-10-14-12-29-05.gh-issue-98257.aMSMs2.rst b/Misc/NEWS.d/next/Library/2022-10-14-12-29-05.gh-issue-98257.aMSMs2.rst new file mode 100644 index 0000000..67f6cb4 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-10-14-12-29-05.gh-issue-98257.aMSMs2.rst @@ -0,0 +1,3 @@ +Make :func:`sys.setprofile` and :func:`sys.settrace` functions reentrant. They +can no long fail with: ``RuntimeError("Cannot install a trace function while +another trace function is being installed")``. Patch by Victor Stinner. diff --git a/Python/ceval.c b/Python/ceval.c index a112f8b..fb8dd48 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -6343,38 +6343,23 @@ _PyEval_SetProfile(PyThreadState *tstate, Py_tracefunc func, PyObject *arg) /* The caller must hold the GIL */ assert(PyGILState_Check()); - static int reentrant = 0; - if (reentrant) { - _PyErr_SetString(tstate, PyExc_RuntimeError, "Cannot install a profile function " - "while another profile function is being installed"); - reentrant = 0; - return -1; - } - reentrant = 1; - /* Call _PySys_Audit() in the context of the current thread state, even if tstate is not the current thread state. */ PyThreadState *current_tstate = _PyThreadState_GET(); if (_PySys_Audit(current_tstate, "sys.setprofile", NULL) < 0) { - reentrant = 0; return -1; } - PyObject *profileobj = tstate->c_profileobj; - - tstate->c_profilefunc = NULL; - tstate->c_profileobj = NULL; - /* Must make sure that tracing is not ignored if 'profileobj' is freed */ - _PyThreadState_UpdateTracingState(tstate); - Py_XDECREF(profileobj); - - Py_XINCREF(arg); - tstate->c_profileobj = arg; tstate->c_profilefunc = func; - + PyObject *old_profileobj = tstate->c_profileobj; + tstate->c_profileobj = Py_XNewRef(arg); /* Flag that tracing or profiling is turned on */ _PyThreadState_UpdateTracingState(tstate); - reentrant = 0; + + // gh-98257: Only call Py_XDECREF() once the new profile function is fully + // set, so it's safe to call sys.setprofile() again (reentrant call). + Py_XDECREF(old_profileobj); + return 0; } @@ -6416,39 +6401,23 @@ _PyEval_SetTrace(PyThreadState *tstate, Py_tracefunc func, PyObject *arg) /* The caller must hold the GIL */ assert(PyGILState_Check()); - static int reentrant = 0; - - if (reentrant) { - _PyErr_SetString(tstate, PyExc_RuntimeError, "Cannot install a trace function " - "while another trace function is being installed"); - reentrant = 0; - return -1; - } - reentrant = 1; - /* Call _PySys_Audit() in the context of the current thread state, even if tstate is not the current thread state. */ PyThreadState *current_tstate = _PyThreadState_GET(); if (_PySys_Audit(current_tstate, "sys.settrace", NULL) < 0) { - reentrant = 0; return -1; } - PyObject *traceobj = tstate->c_traceobj; - - tstate->c_tracefunc = NULL; - tstate->c_traceobj = NULL; - /* Must make sure that profiling is not ignored if 'traceobj' is freed */ - _PyThreadState_UpdateTracingState(tstate); - Py_XINCREF(arg); - Py_XDECREF(traceobj); - tstate->c_traceobj = arg; tstate->c_tracefunc = func; - + PyObject *old_traceobj = tstate->c_traceobj; + tstate->c_traceobj = Py_XNewRef(arg); /* Flag that tracing or profiling is turned on */ _PyThreadState_UpdateTracingState(tstate); - reentrant = 0; + // gh-98257: Only call Py_XDECREF() once the new trace function is fully + // set, so it's safe to call sys.settrace() again (reentrant call). + Py_XDECREF(old_traceobj); + return 0; } -- cgit v0.12