diff options
author | Victor Stinner <vstinner@python.org> | 2022-10-19 22:31:47 (GMT) |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-10-19 22:31:47 (GMT) |
commit | a8fe4bbd6b78517f640e25697338b9448c4675c1 (patch) | |
tree | b661e5d047c6a0a552c5ccf132cad0df636ba4e1 | |
parent | 4bd63f66cd4f6e8d549f88ae0f4b0106d522b6bb (diff) | |
download | cpython-a8fe4bbd6b78517f640e25697338b9448c4675c1.zip cpython-a8fe4bbd6b78517f640e25697338b9448c4675c1.tar.gz cpython-a8fe4bbd6b78517f640e25697338b9448c4675c1.tar.bz2 |
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.
-rw-r--r-- | Lib/test/test_sys_setprofile.py | 8 | ||||
-rw-r--r-- | Lib/test/test_sys_settrace.py | 8 | ||||
-rw-r--r-- | Misc/NEWS.d/next/Library/2022-10-14-12-29-05.gh-issue-98257.aMSMs2.rst | 3 | ||||
-rw-r--r-- | Python/ceval.c | 57 |
4 files changed, 20 insertions, 56 deletions
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; } |