[Python-checkins] gh-98257: Make _PyEval_SetTrace() reentrant (#98258)

[vstinner]

https://github.com/python/cpython/commit/a8fe4bbd6b78517f640e25697338b9448c4675c1
commit: a8fe4bbd6b78517f640e25697338b9448c4675c1
branch: main
author: Victor Stinner <vstinner at python.org>
committer: vstinner <vstinner at python.org>
date: 2022-10-20T00:31:47+02:00
summary:

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.

files:
A Misc/NEWS.d/next/Library/2022-10-14-12-29-05.gh-issue-98257.aMSMs2.rst
M Lib/test/test_sys_setprofile.py
M Lib/test/test_sys_settrace.py
M Python/ceval.c

diff --git a/Lib/test/test_sys_setprofile.py b/Lib/test/test_sys_setprofile.py
index 4c3053a1e3e9..acae433cd0a5 100644
--- a/Lib/test/test_sys_setprofile.py
+++ b/Lib/test/test_sys_setprofile.py
@@ -437,12 +437,8 @@ def __del__(self):
                 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 4f577c2ea5da..a448f80449ca 100644
--- a/Lib/test/test_sys_settrace.py
+++ b/Lib/test/test_sys_settrace.py
@@ -2796,12 +2796,8 @@ def __del__(self):
                 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 000000000000..67f6cb4dbc19
--- /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 a112f8bebb5a..fb8dd4811628 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;
 }