From 4c8fa531aaecf282336bf24b9c17c2fea4034af0 Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Wed, 13 Aug 2025 14:15:12 -0400 Subject: [PATCH 1/2] [3.13] gh-137400: Fix thread-safety issues when profiling all threads (gh-137518) There were a few thread-safety issues when profiling or tracing all threads via PyEval_SetProfileAllThreads or PyEval_SetTraceAllThreads: * The loop over thread states could crash if a thread exits concurrently (in both the free threading and default build) * The modification of `c_profilefunc` and `c_tracefunc` wasn't thread-safe on the free threading build. (cherry picked from commit a10152f8fd0f4b291e53d646cffe22fbeec73e1e) Co-authored-by: Sam Gross --- Include/internal/pycore_ceval.h | 2 + Include/internal/pycore_interp.h | 4 +- .../test_free_threading/test_monitoring.py | 26 ++ ...-08-07-09-52-19.gh-issue-137400.AK1dy-.rst | 5 + Python/bytecodes.c | 10 +- Python/ceval.c | 38 +- Python/instrumentation.c | 67 ++- Python/legacy_tracing.c | 433 ++++++++++++------ Python/pystate.c | 9 +- Python/sysmodule.c | 13 +- 10 files changed, 379 insertions(+), 228 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2025-08-07-09-52-19.gh-issue-137400.AK1dy-.rst diff --git a/Include/internal/pycore_ceval.h b/Include/internal/pycore_ceval.h index 043f5957d481e5..f804823cc530dd 100644 --- a/Include/internal/pycore_ceval.h +++ b/Include/internal/pycore_ceval.h @@ -19,8 +19,10 @@ struct _ceval_runtime_state; // Export for '_lsprof' shared extension PyAPI_FUNC(int) _PyEval_SetProfile(PyThreadState *tstate, Py_tracefunc func, PyObject *arg); +extern int _PyEval_SetProfileAllThreads(PyInterpreterState *interp, Py_tracefunc func, PyObject *arg); extern int _PyEval_SetTrace(PyThreadState *tstate, Py_tracefunc func, PyObject *arg); +extern int _PyEval_SetTraceAllThreads(PyInterpreterState *interp, Py_tracefunc func, PyObject *arg); extern int _PyEval_SetOpcodeTrace(PyFrameObject *f, bool enable); diff --git a/Include/internal/pycore_interp.h b/Include/internal/pycore_interp.h index 075d35a3755777..3243b1e618dd56 100644 --- a/Include/internal/pycore_interp.h +++ b/Include/internal/pycore_interp.h @@ -262,8 +262,8 @@ struct _is { PyDict_WatchCallback builtins_dict_watcher; _Py_GlobalMonitors monitors; - bool sys_profile_initialized; - bool sys_trace_initialized; + _PyOnceFlag sys_profile_once_flag; + _PyOnceFlag sys_trace_once_flag; Py_ssize_t sys_profiling_threads; /* Count of threads with c_profilefunc set */ Py_ssize_t sys_tracing_threads; /* Count of threads with c_tracefunc set */ PyObject *monitoring_callables[PY_MONITORING_TOOL_IDS][_PY_MONITORING_EVENTS]; diff --git a/Lib/test/test_free_threading/test_monitoring.py b/Lib/test/test_free_threading/test_monitoring.py index 51860a7c94539f..35aaea76a1912c 100644 --- a/Lib/test/test_free_threading/test_monitoring.py +++ b/Lib/test/test_free_threading/test_monitoring.py @@ -193,6 +193,32 @@ def during_threads(self): self.set = not self.set +@threading_helper.requires_working_threading() +class SetProfileAllThreadsMultiThreaded(InstrumentationMultiThreadedMixin, TestCase): + """Uses threading.setprofile_all_threads and repeatedly toggles instrumentation on and off""" + + def setUp(self): + self.set = False + self.called = False + + def after_test(self): + self.assertTrue(self.called) + + def tearDown(self): + threading.setprofile_all_threads(None) + + def trace_func(self, frame, event, arg): + self.called = True + return self.trace_func + + def during_threads(self): + if self.set: + threading.setprofile_all_threads(self.trace_func) + else: + threading.setprofile_all_threads(None) + self.set = not self.set + + @threading_helper.requires_working_threading() class SetProfileAllMultiThreaded(TestCase): def test_profile_all_threads(self): diff --git a/Misc/NEWS.d/next/Core and Builtins/2025-08-07-09-52-19.gh-issue-137400.AK1dy-.rst b/Misc/NEWS.d/next/Core and Builtins/2025-08-07-09-52-19.gh-issue-137400.AK1dy-.rst new file mode 100644 index 00000000000000..406d6528840ba5 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2025-08-07-09-52-19.gh-issue-137400.AK1dy-.rst @@ -0,0 +1,5 @@ +Fix a crash in the :term:`free threading` build when disabling profiling or +tracing across all threads with :c:func:`PyEval_SetProfileAllThreads` or +:c:func:`PyEval_SetTraceAllThreads` or their Python equivalents +:func:`threading.settrace_all_threads` and +:func:`threading.setprofile_all_threads`. diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 1da434bbbc892a..152f8f1d60d45c 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -148,7 +148,15 @@ dummy_func( tier1 inst(RESUME, (--)) { assert(frame == tstate->current_frame); - if (tstate->tracing == 0) { + #ifdef Py_GIL_DISABLED + // For thread-safety, we need to check instrumentation version + // even when tracing. Otherwise, another thread may concurrently + // re-write the bytecode while we are executing this function. + int check_instrumentation = 1; + #else + int check_instrumentation = (tstate->tracing == 0); + #endif + if (check_instrumentation) { uintptr_t global_version = _Py_atomic_load_uintptr_relaxed(&tstate->eval_breaker) & ~_PY_EVAL_EVENTS_MASK; diff --git a/Python/ceval.c b/Python/ceval.c index 8c0cb29863cc60..98a0b25d6d1d5b 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -2334,21 +2334,10 @@ PyEval_SetProfile(Py_tracefunc func, PyObject *arg) void PyEval_SetProfileAllThreads(Py_tracefunc func, PyObject *arg) { - PyThreadState *this_tstate = _PyThreadState_GET(); - PyInterpreterState* interp = this_tstate->interp; - - _PyRuntimeState *runtime = &_PyRuntime; - HEAD_LOCK(runtime); - PyThreadState* ts = PyInterpreterState_ThreadHead(interp); - HEAD_UNLOCK(runtime); - - while (ts) { - if (_PyEval_SetProfile(ts, func, arg) < 0) { - PyErr_FormatUnraisable("Exception ignored in PyEval_SetProfileAllThreads"); - } - HEAD_LOCK(runtime); - ts = PyThreadState_Next(ts); - HEAD_UNLOCK(runtime); + PyInterpreterState *interp = _PyInterpreterState_GET(); + if (_PyEval_SetProfileAllThreads(interp, func, arg) < 0) { + /* Log _PySys_Audit() error */ + PyErr_FormatUnraisable("Exception ignored in PyEval_SetProfileAllThreads"); } } @@ -2365,21 +2354,10 @@ PyEval_SetTrace(Py_tracefunc func, PyObject *arg) void PyEval_SetTraceAllThreads(Py_tracefunc func, PyObject *arg) { - PyThreadState *this_tstate = _PyThreadState_GET(); - PyInterpreterState* interp = this_tstate->interp; - - _PyRuntimeState *runtime = &_PyRuntime; - HEAD_LOCK(runtime); - PyThreadState* ts = PyInterpreterState_ThreadHead(interp); - HEAD_UNLOCK(runtime); - - while (ts) { - if (_PyEval_SetTrace(ts, func, arg) < 0) { - PyErr_FormatUnraisable("Exception ignored in PyEval_SetTraceAllThreads"); - } - HEAD_LOCK(runtime); - ts = PyThreadState_Next(ts); - HEAD_UNLOCK(runtime); + PyInterpreterState *interp = _PyInterpreterState_GET(); + if (_PyEval_SetTraceAllThreads(interp, func, arg) < 0) { + /* Log _PySys_Audit() error */ + PyErr_FormatUnraisable("Exception ignored in PyEval_SetTraceAllThreads"); } } diff --git a/Python/instrumentation.c b/Python/instrumentation.c index 956d50b6a7c0b2..5f10cdfc7de989 100644 --- a/Python/instrumentation.c +++ b/Python/instrumentation.c @@ -1012,6 +1012,8 @@ set_version_raw(uintptr_t *ptr, uint32_t version) static void set_global_version(PyThreadState *tstate, uint32_t version) { + ASSERT_WORLD_STOPPED(); + assert((version & _PY_EVAL_EVENTS_MASK) == 0); PyInterpreterState *interp = tstate->interp; set_version_raw(&interp->ceval.instrumentation_version, version); @@ -1908,28 +1910,27 @@ _Py_Instrument(PyCodeObject *code, PyInterpreterState *interp) static int -instrument_all_executing_code_objects(PyInterpreterState *interp) { +instrument_all_executing_code_objects(PyInterpreterState *interp) +{ ASSERT_WORLD_STOPPED(); - _PyRuntimeState *runtime = &_PyRuntime; - HEAD_LOCK(runtime); - PyThreadState* ts = PyInterpreterState_ThreadHead(interp); - HEAD_UNLOCK(runtime); - while (ts) { + int err = 0; + HEAD_LOCK(&_PyRuntime); + for (PyThreadState *ts = interp->threads.head; ts != NULL; ts = ts->next) { _PyInterpreterFrame *frame = ts->current_frame; while (frame) { if (frame->owner != FRAME_OWNED_BY_CSTACK) { - if (instrument_lock_held(_PyFrame_GetCode(frame), interp)) { - return -1; + err = instrument_lock_held(_PyFrame_GetCode(frame), interp); + if (err) { + goto done; } } frame = frame->previous; } - HEAD_LOCK(runtime); - ts = PyThreadState_Next(ts); - HEAD_UNLOCK(runtime); } - return 0; +done: + HEAD_UNLOCK(&_PyRuntime); + return err; } static void @@ -1975,6 +1976,7 @@ check_tool(PyInterpreterState *interp, int tool_id) int _PyMonitoring_SetEvents(int tool_id, _PyMonitoringEventSet events) { + ASSERT_WORLD_STOPPED(); assert(0 <= tool_id && tool_id < PY_MONITORING_TOOL_IDS); PyThreadState *tstate = _PyThreadState_GET(); PyInterpreterState *interp = tstate->interp; @@ -1983,33 +1985,28 @@ _PyMonitoring_SetEvents(int tool_id, _PyMonitoringEventSet events) return -1; } - int res; - _PyEval_StopTheWorld(interp); uint32_t existing_events = get_events(&interp->monitors, tool_id); if (existing_events == events) { - res = 0; - goto done; + return 0; } set_events(&interp->monitors, tool_id, events); uint32_t new_version = global_version(interp) + MONITORING_VERSION_INCREMENT; if (new_version == 0) { PyErr_Format(PyExc_OverflowError, "events set too many times"); - res = -1; - goto done; + return -1; } set_global_version(tstate, new_version); #ifdef _Py_TIER2 _Py_Executors_InvalidateAll(interp, 1); #endif - res = instrument_all_executing_code_objects(interp); -done: - _PyEval_StartTheWorld(interp); - return res; + return instrument_all_executing_code_objects(interp); } int _PyMonitoring_SetLocalEvents(PyCodeObject *code, int tool_id, _PyMonitoringEventSet events) { + ASSERT_WORLD_STOPPED(); + assert(0 <= tool_id && tool_id < PY_MONITORING_TOOL_IDS); PyInterpreterState *interp = _PyInterpreterState_GET(); assert(events < (1 << _PY_MONITORING_LOCAL_EVENTS)); @@ -2021,26 +2018,18 @@ _PyMonitoring_SetLocalEvents(PyCodeObject *code, int tool_id, _PyMonitoringEvent return -1; } - int res; - _PyEval_StopTheWorld(interp); if (allocate_instrumentation_data(code)) { - res = -1; - goto done; + return -1; } _Py_LocalMonitors *local = &code->_co_monitoring->local_monitors; uint32_t existing_events = get_local_events(local, tool_id); if (existing_events == events) { - res = 0; - goto done; + return 0; } set_local_events(local, tool_id, events); - res = force_instrument_lock_held(code, interp); - -done: - _PyEval_StartTheWorld(interp); - return res; + return force_instrument_lock_held(code, interp); } int @@ -2238,7 +2227,11 @@ monitoring_set_events_impl(PyObject *module, int tool_id, int event_set) return NULL; } event_set &= ~C_RETURN_EVENTS; - if (_PyMonitoring_SetEvents(tool_id, event_set)) { + PyInterpreterState *interp = _PyInterpreterState_GET(); + _PyEval_StopTheWorld(interp); + int err = _PyMonitoring_SetEvents(tool_id, event_set); + _PyEval_StartTheWorld(interp); + if (err) { return NULL; } Py_RETURN_NONE; @@ -2315,7 +2308,11 @@ monitoring_set_local_events_impl(PyObject *module, int tool_id, return NULL; } - if (_PyMonitoring_SetLocalEvents((PyCodeObject*)code, tool_id, event_set)) { + PyInterpreterState *interp = _PyInterpreterState_GET(); + _PyEval_StopTheWorld(interp); + int err = _PyMonitoring_SetLocalEvents((PyCodeObject*)code, tool_id, event_set); + _PyEval_StartTheWorld(interp); + if (err) { return NULL; } Py_RETURN_NONE; diff --git a/Python/legacy_tracing.c b/Python/legacy_tracing.c index 2f6d43658a9fa4..d7a195cf166895 100644 --- a/Python/legacy_tracing.c +++ b/Python/legacy_tracing.c @@ -137,17 +137,10 @@ sys_profile_call_or_return( Py_RETURN_NONE; } -int -_PyEval_SetOpcodeTrace( - PyFrameObject *frame, - bool enable -) { - assert(frame != NULL); - assert(PyCode_Check(frame->f_frame->f_executable)); - - PyCodeObject *code = (PyCodeObject *)frame->f_frame->f_executable; +static int +set_opcode_trace_world_stopped(PyCodeObject *code, bool enable) +{ _PyMonitoringEventSet events = 0; - if (_PyMonitoring_GetLocalEvents(code, PY_MONITORING_SYS_TRACE_ID, &events) < 0) { return -1; } @@ -166,6 +159,32 @@ _PyEval_SetOpcodeTrace( return _PyMonitoring_SetLocalEvents(code, PY_MONITORING_SYS_TRACE_ID, events); } +int +_PyEval_SetOpcodeTrace(PyFrameObject *frame, bool enable) +{ + assert(frame != NULL); + + PyCodeObject *code = _PyFrame_GetCode(frame->f_frame); + +#ifdef Py_GIL_DISABLED + // First check if a change is necessary outside of the stop-the-world pause + _PyMonitoringEventSet events = 0; + if (_PyMonitoring_GetLocalEvents(code, PY_MONITORING_SYS_TRACE_ID, &events) < 0) { + return -1; + } + int is_enabled = (events & (1 << PY_MONITORING_EVENT_INSTRUCTION)) != 0; + if (is_enabled == enable) { + return 0; // No change needed + } +#endif + + PyInterpreterState *interp = _PyInterpreterState_GET(); + _PyEval_StopTheWorld(interp); + int res = set_opcode_trace_world_stopped(code, enable); + _PyEval_StartTheWorld(interp); + return res; +} + static PyObject * call_trace_func(_PyLegacyEventHandler *self, PyObject *arg) { @@ -435,60 +454,72 @@ is_tstate_valid(PyThreadState *tstate) } #endif -static Py_ssize_t -setup_profile(PyThreadState *tstate, Py_tracefunc func, PyObject *arg, PyObject **old_profileobj) +static int +setup_profile_callbacks(void *Py_UNUSED(arg)) { - *old_profileobj = NULL; /* Setup PEP 669 monitoring callbacks and events. */ - if (!tstate->interp->sys_profile_initialized) { - tstate->interp->sys_profile_initialized = true; - if (set_callbacks(PY_MONITORING_SYS_PROFILE_ID, - (vectorcallfunc)sys_profile_start, PyTrace_CALL, - PY_MONITORING_EVENT_PY_START, PY_MONITORING_EVENT_PY_RESUME)) { - return -1; - } - if (set_callbacks(PY_MONITORING_SYS_PROFILE_ID, - (vectorcallfunc)sys_profile_throw, PyTrace_CALL, - PY_MONITORING_EVENT_PY_THROW, -1)) { - return -1; - } - if (set_callbacks(PY_MONITORING_SYS_PROFILE_ID, - (vectorcallfunc)sys_profile_return, PyTrace_RETURN, - PY_MONITORING_EVENT_PY_RETURN, PY_MONITORING_EVENT_PY_YIELD)) { - return -1; - } - if (set_callbacks(PY_MONITORING_SYS_PROFILE_ID, - (vectorcallfunc)sys_profile_unwind, PyTrace_RETURN, - PY_MONITORING_EVENT_PY_UNWIND, -1)) { - return -1; - } - if (set_callbacks(PY_MONITORING_SYS_PROFILE_ID, - (vectorcallfunc)sys_profile_call_or_return, PyTrace_C_CALL, - PY_MONITORING_EVENT_CALL, -1)) { - return -1; - } - if (set_callbacks(PY_MONITORING_SYS_PROFILE_ID, - (vectorcallfunc)sys_profile_call_or_return, PyTrace_C_RETURN, - PY_MONITORING_EVENT_C_RETURN, -1)) { - return -1; - } - if (set_callbacks(PY_MONITORING_SYS_PROFILE_ID, - (vectorcallfunc)sys_profile_call_or_return, PyTrace_C_EXCEPTION, - PY_MONITORING_EVENT_C_RAISE, -1)) { - return -1; - } + if (set_callbacks(PY_MONITORING_SYS_PROFILE_ID, + (vectorcallfunc)sys_profile_start, PyTrace_CALL, + PY_MONITORING_EVENT_PY_START, PY_MONITORING_EVENT_PY_RESUME)) { + return -1; + } + if (set_callbacks(PY_MONITORING_SYS_PROFILE_ID, + (vectorcallfunc)sys_profile_throw, PyTrace_CALL, + PY_MONITORING_EVENT_PY_THROW, -1)) { + return -1; + } + if (set_callbacks(PY_MONITORING_SYS_PROFILE_ID, + (vectorcallfunc)sys_profile_return, PyTrace_RETURN, + PY_MONITORING_EVENT_PY_RETURN, PY_MONITORING_EVENT_PY_YIELD)) { + return -1; + } + if (set_callbacks(PY_MONITORING_SYS_PROFILE_ID, + (vectorcallfunc)sys_profile_unwind, PyTrace_RETURN, + PY_MONITORING_EVENT_PY_UNWIND, -1)) { + return -1; + } + if (set_callbacks(PY_MONITORING_SYS_PROFILE_ID, + (vectorcallfunc)sys_profile_call_or_return, PyTrace_C_CALL, + PY_MONITORING_EVENT_CALL, -1)) { + return -1; + } + if (set_callbacks(PY_MONITORING_SYS_PROFILE_ID, + (vectorcallfunc)sys_profile_call_or_return, PyTrace_C_RETURN, + PY_MONITORING_EVENT_C_RETURN, -1)) { + return -1; + } + if (set_callbacks(PY_MONITORING_SYS_PROFILE_ID, + (vectorcallfunc)sys_profile_call_or_return, PyTrace_C_EXCEPTION, + PY_MONITORING_EVENT_C_RAISE, -1)) { + return -1; } +return 0; +} - _PyEval_StopTheWorld(tstate->interp); +static PyObject * +swap_profile_func_arg(PyThreadState *tstate, Py_tracefunc func, PyObject *arg) +{ int delta = (func != NULL) - (tstate->c_profilefunc != NULL); tstate->c_profilefunc = func; - *old_profileobj = tstate->c_profileobj; + PyObject *old_profileobj = tstate->c_profileobj; tstate->c_profileobj = Py_XNewRef(arg); tstate->interp->sys_profiling_threads += delta; assert(tstate->interp->sys_profiling_threads >= 0); - Py_ssize_t profiling_threads = tstate->interp->sys_profiling_threads; - _PyEval_StartTheWorld(tstate->interp); - return profiling_threads; + return old_profileobj; +} + +static int +set_monitoring_profile_events(PyInterpreterState *interp) +{ + uint32_t events = 0; + if (interp->sys_profiling_threads) { + events = + (1 << PY_MONITORING_EVENT_PY_START) | (1 << PY_MONITORING_EVENT_PY_RESUME) | + (1 << PY_MONITORING_EVENT_PY_RETURN) | (1 << PY_MONITORING_EVENT_PY_YIELD) | + (1 << PY_MONITORING_EVENT_CALL) | (1 << PY_MONITORING_EVENT_PY_UNWIND) | + (1 << PY_MONITORING_EVENT_PY_THROW); + } + return _PyMonitoring_SetEvents(PY_MONITORING_SYS_PROFILE_ID, events); } int @@ -505,88 +536,153 @@ _PyEval_SetProfile(PyThreadState *tstate, Py_tracefunc func, PyObject *arg) return -1; } - // needs to be decref'd outside of the lock - PyObject *old_profileobj; - LOCK_SETUP(); - Py_ssize_t profiling_threads = setup_profile(tstate, func, arg, &old_profileobj); - UNLOCK_SETUP(); - Py_XDECREF(old_profileobj); + PyInterpreterState *interp = tstate->interp; + if (_PyOnceFlag_CallOnce(&interp->sys_profile_once_flag, + setup_profile_callbacks, NULL) < 0) { + return -1; + } - uint32_t events = 0; - if (profiling_threads) { - events = - (1 << PY_MONITORING_EVENT_PY_START) | (1 << PY_MONITORING_EVENT_PY_RESUME) | - (1 << PY_MONITORING_EVENT_PY_RETURN) | (1 << PY_MONITORING_EVENT_PY_YIELD) | - (1 << PY_MONITORING_EVENT_CALL) | (1 << PY_MONITORING_EVENT_PY_UNWIND) | - (1 << PY_MONITORING_EVENT_PY_THROW); + _PyEval_StopTheWorld(interp); + PyObject *old_profileobj = swap_profile_func_arg(tstate, func, arg); + int ret = set_monitoring_profile_events(interp); + _PyEval_StartTheWorld(interp); + Py_XDECREF(old_profileobj); // needs to be decref'd outside of stop-the-world + return ret; +} + +int +_PyEval_SetProfileAllThreads(PyInterpreterState *interp, Py_tracefunc func, PyObject *arg) +{ + PyThreadState *current_tstate = _PyThreadState_GET(); + assert(is_tstate_valid(current_tstate)); + assert(current_tstate->interp == interp); + + if (_PySys_Audit(current_tstate, "sys.setprofile", NULL) < 0) { + return -1; } - return _PyMonitoring_SetEvents(PY_MONITORING_SYS_PROFILE_ID, events); + + if (_PyOnceFlag_CallOnce(&interp->sys_profile_once_flag, + setup_profile_callbacks, NULL) < 0) { + return -1; + } + + PyObject *old_profileobjs = NULL; + _PyEval_StopTheWorld(interp); + HEAD_LOCK(&_PyRuntime); + Py_ssize_t num_thread_states = 0; + for (PyThreadState *p = interp->threads.head; p; p = p->next) { + num_thread_states++; + } + old_profileobjs = PyTuple_New(num_thread_states); + if (old_profileobjs == NULL) { + HEAD_UNLOCK(&_PyRuntime); + _PyEval_StartTheWorld(interp); + return -1; + } + for (PyThreadState *tstate = interp->threads.head; tstate; tstate = tstate->next) { + PyObject *old = swap_profile_func_arg(tstate, func, arg); + PyTuple_SET_ITEM(old_profileobjs, --num_thread_states, old); + } + HEAD_UNLOCK(&_PyRuntime); + int ret = set_monitoring_profile_events(interp); + _PyEval_StartTheWorld(interp); + Py_XDECREF(old_profileobjs); // needs to be decref'd outside of stop-the-world + return ret; } -static Py_ssize_t -setup_tracing(PyThreadState *tstate, Py_tracefunc func, PyObject *arg, PyObject **old_traceobj) +static int +setup_trace_callbacks(void *Py_UNUSED(arg)) { - *old_traceobj = NULL; /* Setup PEP 669 monitoring callbacks and events. */ - if (!tstate->interp->sys_trace_initialized) { - tstate->interp->sys_trace_initialized = true; - if (set_callbacks(PY_MONITORING_SYS_TRACE_ID, - (vectorcallfunc)sys_trace_start, PyTrace_CALL, - PY_MONITORING_EVENT_PY_START, PY_MONITORING_EVENT_PY_RESUME)) { - return -1; - } - if (set_callbacks(PY_MONITORING_SYS_TRACE_ID, - (vectorcallfunc)sys_trace_throw, PyTrace_CALL, - PY_MONITORING_EVENT_PY_THROW, -1)) { - return -1; - } - if (set_callbacks(PY_MONITORING_SYS_TRACE_ID, - (vectorcallfunc)sys_trace_return, PyTrace_RETURN, - PY_MONITORING_EVENT_PY_RETURN, -1)) { - return -1; - } - if (set_callbacks(PY_MONITORING_SYS_TRACE_ID, - (vectorcallfunc)sys_trace_yield, PyTrace_RETURN, - PY_MONITORING_EVENT_PY_YIELD, -1)) { - return -1; - } - if (set_callbacks(PY_MONITORING_SYS_TRACE_ID, - (vectorcallfunc)sys_trace_exception_func, PyTrace_EXCEPTION, - PY_MONITORING_EVENT_RAISE, PY_MONITORING_EVENT_STOP_ITERATION)) { - return -1; - } - if (set_callbacks(PY_MONITORING_SYS_TRACE_ID, - (vectorcallfunc)sys_trace_line_func, PyTrace_LINE, - PY_MONITORING_EVENT_LINE, -1)) { - return -1; - } - if (set_callbacks(PY_MONITORING_SYS_TRACE_ID, - (vectorcallfunc)sys_trace_unwind, PyTrace_RETURN, - PY_MONITORING_EVENT_PY_UNWIND, -1)) { - return -1; - } - if (set_callbacks(PY_MONITORING_SYS_TRACE_ID, - (vectorcallfunc)sys_trace_jump_func, PyTrace_LINE, - PY_MONITORING_EVENT_JUMP, -1)) { - return -1; - } - if (set_callbacks(PY_MONITORING_SYS_TRACE_ID, - (vectorcallfunc)sys_trace_instruction_func, PyTrace_OPCODE, - PY_MONITORING_EVENT_INSTRUCTION, -1)) { - return -1; - } + if (set_callbacks(PY_MONITORING_SYS_TRACE_ID, + (vectorcallfunc)sys_trace_start, PyTrace_CALL, + PY_MONITORING_EVENT_PY_START, PY_MONITORING_EVENT_PY_RESUME)) { + return -1; + } + if (set_callbacks(PY_MONITORING_SYS_TRACE_ID, + (vectorcallfunc)sys_trace_throw, PyTrace_CALL, + PY_MONITORING_EVENT_PY_THROW, -1)) { + return -1; + } + if (set_callbacks(PY_MONITORING_SYS_TRACE_ID, + (vectorcallfunc)sys_trace_return, PyTrace_RETURN, + PY_MONITORING_EVENT_PY_RETURN, -1)) { + return -1; + } + if (set_callbacks(PY_MONITORING_SYS_TRACE_ID, + (vectorcallfunc)sys_trace_yield, PyTrace_RETURN, + PY_MONITORING_EVENT_PY_YIELD, -1)) { + return -1; + } + if (set_callbacks(PY_MONITORING_SYS_TRACE_ID, + (vectorcallfunc)sys_trace_exception_func, PyTrace_EXCEPTION, + PY_MONITORING_EVENT_RAISE, PY_MONITORING_EVENT_STOP_ITERATION)) { + return -1; } + if (set_callbacks(PY_MONITORING_SYS_TRACE_ID, + (vectorcallfunc)sys_trace_line_func, PyTrace_LINE, + PY_MONITORING_EVENT_LINE, -1)) { + return -1; + } + if (set_callbacks(PY_MONITORING_SYS_TRACE_ID, + (vectorcallfunc)sys_trace_unwind, PyTrace_RETURN, + PY_MONITORING_EVENT_PY_UNWIND, -1)) { + return -1; + } + if (set_callbacks(PY_MONITORING_SYS_TRACE_ID, + (vectorcallfunc)sys_trace_jump_func, PyTrace_LINE, + PY_MONITORING_EVENT_JUMP, -1)) { + return -1; + } + if (set_callbacks(PY_MONITORING_SYS_TRACE_ID, + (vectorcallfunc)sys_trace_instruction_func, PyTrace_OPCODE, + PY_MONITORING_EVENT_INSTRUCTION, -1)) { + return -1; + } + return 0; +} - _PyEval_StopTheWorld(tstate->interp); +static PyObject * +swap_trace_func_arg(PyThreadState *tstate, Py_tracefunc func, PyObject *arg) +{ int delta = (func != NULL) - (tstate->c_tracefunc != NULL); tstate->c_tracefunc = func; - *old_traceobj = tstate->c_traceobj; + PyObject *old_traceobj = tstate->c_traceobj; tstate->c_traceobj = Py_XNewRef(arg); tstate->interp->sys_tracing_threads += delta; assert(tstate->interp->sys_tracing_threads >= 0); - Py_ssize_t tracing_threads = tstate->interp->sys_tracing_threads; - _PyEval_StartTheWorld(tstate->interp); - return tracing_threads; + return old_traceobj; +} + +static int +set_monitoring_trace_events(PyInterpreterState *interp) +{ + uint32_t events = 0; + if (interp->sys_tracing_threads) { + events = + (1 << PY_MONITORING_EVENT_PY_START) | (1 << PY_MONITORING_EVENT_PY_RESUME) | + (1 << PY_MONITORING_EVENT_PY_RETURN) | (1 << PY_MONITORING_EVENT_PY_YIELD) | + (1 << PY_MONITORING_EVENT_RAISE) | (1 << PY_MONITORING_EVENT_LINE) | + (1 << PY_MONITORING_EVENT_JUMP) | + (1 << PY_MONITORING_EVENT_PY_UNWIND) | (1 << PY_MONITORING_EVENT_PY_THROW) | + (1 << PY_MONITORING_EVENT_STOP_ITERATION); + } + return _PyMonitoring_SetEvents(PY_MONITORING_SYS_TRACE_ID, events); +} + +// Enable opcode tracing for the thread's current frame if needed. +static int +maybe_set_opcode_trace(PyThreadState *tstate) +{ + _PyInterpreterFrame *iframe = tstate->current_frame; + if (iframe == NULL) { + return 0; + } + PyFrameObject *frame = iframe->frame_obj; + if (frame == NULL || !frame->f_trace_opcodes) { + return 0; + } + return set_opcode_trace_world_stopped(_PyFrame_GetCode(iframe), true); } int @@ -602,35 +698,76 @@ _PyEval_SetTrace(PyThreadState *tstate, Py_tracefunc func, PyObject *arg) if (_PySys_Audit(current_tstate, "sys.settrace", NULL) < 0) { return -1; } - // needs to be decref'd outside of the lock - PyObject *old_traceobj; - LOCK_SETUP(); - assert(tstate->interp->sys_tracing_threads >= 0); - Py_ssize_t tracing_threads = setup_tracing(tstate, func, arg, &old_traceobj); - UNLOCK_SETUP(); - Py_XDECREF(old_traceobj); - if (tracing_threads < 0) { + + PyInterpreterState *interp = tstate->interp; + if (_PyOnceFlag_CallOnce(&interp->sys_trace_once_flag, + setup_trace_callbacks, NULL) < 0) { return -1; } - uint32_t events = 0; - if (tracing_threads) { - events = - (1 << PY_MONITORING_EVENT_PY_START) | (1 << PY_MONITORING_EVENT_PY_RESUME) | - (1 << PY_MONITORING_EVENT_PY_RETURN) | (1 << PY_MONITORING_EVENT_PY_YIELD) | - (1 << PY_MONITORING_EVENT_RAISE) | (1 << PY_MONITORING_EVENT_LINE) | - (1 << PY_MONITORING_EVENT_JUMP) | - (1 << PY_MONITORING_EVENT_PY_UNWIND) | (1 << PY_MONITORING_EVENT_PY_THROW) | - (1 << PY_MONITORING_EVENT_STOP_ITERATION); + int err = 0; + _PyEval_StopTheWorld(interp); + PyObject *old_traceobj = swap_trace_func_arg(tstate, func, arg); + err = set_monitoring_trace_events(interp); + if (err != 0) { + goto done; + } + if (interp->sys_tracing_threads) { + err = maybe_set_opcode_trace(tstate); + } +done: + _PyEval_StartTheWorld(interp); + Py_XDECREF(old_traceobj); // needs to be decref'd outside stop-the-world + return err; +} + +int +_PyEval_SetTraceAllThreads(PyInterpreterState *interp, Py_tracefunc func, PyObject *arg) +{ + PyThreadState *current_tstate = _PyThreadState_GET(); + assert(is_tstate_valid(current_tstate)); + assert(current_tstate->interp == interp); + + if (_PySys_Audit(current_tstate, "sys.settrace", NULL) < 0) { + return -1; + } + + if (_PyOnceFlag_CallOnce(&interp->sys_trace_once_flag, + setup_trace_callbacks, NULL) < 0) { + return -1; + } - PyFrameObject* frame = PyEval_GetFrame(); - if (frame && frame->f_trace_opcodes) { - int ret = _PyEval_SetOpcodeTrace(frame, true); - if (ret != 0) { - return ret; + PyObject *old_trace_objs = NULL; + _PyEval_StopTheWorld(interp); + HEAD_LOCK(&_PyRuntime); + Py_ssize_t num_thread_states = 0; + for (PyThreadState *p = interp->threads.head; p; p = p->next) { + num_thread_states++; + } + old_trace_objs = PyTuple_New(num_thread_states); + if (old_trace_objs == NULL) { + HEAD_UNLOCK(&_PyRuntime); + _PyEval_StartTheWorld(interp); + return -1; + } + for (PyThreadState *tstate = interp->threads.head; tstate; tstate = tstate->next) { + PyObject *old = swap_trace_func_arg(tstate, func, arg); + PyTuple_SET_ITEM(old_trace_objs, --num_thread_states, old); + } + if (interp->sys_tracing_threads) { + for (PyThreadState *tstate = interp->threads.head; tstate; tstate = tstate->next) { + int err = maybe_set_opcode_trace(tstate); + if (err != 0) { + HEAD_UNLOCK(&_PyRuntime); + _PyEval_StartTheWorld(interp); + Py_XDECREF(old_trace_objs); + return -1; } } } - - return _PyMonitoring_SetEvents(PY_MONITORING_SYS_TRACE_ID, events); + HEAD_UNLOCK(&_PyRuntime); + int err = set_monitoring_trace_events(interp); + _PyEval_StartTheWorld(interp); + Py_XDECREF(old_trace_objs); // needs to be decref'd outside of stop-the-world + return err; } diff --git a/Python/pystate.c b/Python/pystate.c index 5222a7ec140528..68207443532cd0 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -399,7 +399,6 @@ _Py_COMP_DIAG_POP &(runtime)->unicode_state.ids.mutex, \ &(runtime)->imports.extensions.mutex, \ &(runtime)->ceval.pending_mainthread.mutex, \ - &(runtime)->ceval.sys_trace_profile_mutex, \ &(runtime)->atexit.mutex, \ &(runtime)->audit_hooks.mutex, \ &(runtime)->allocators.mutex, \ @@ -654,8 +653,6 @@ init_interpreter(PyInterpreterState *interp, } } - interp->sys_profile_initialized = false; - interp->sys_trace_initialized = false; #ifdef _Py_TIER2 (void)_Py_SetOptimizer(interp, NULL); interp->executor_list_head = NULL; @@ -838,8 +835,6 @@ interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate) Py_CLEAR(interp->monitoring_callables[t][e]); } } - interp->sys_profile_initialized = false; - interp->sys_trace_initialized = false; for (int t = 0; t < PY_MONITORING_TOOL_IDS; t++) { Py_CLEAR(interp->monitoring_tool_names[t]); } @@ -1724,11 +1719,11 @@ PyThreadState_Clear(PyThreadState *tstate) } if (tstate->c_profilefunc != NULL) { - tstate->interp->sys_profiling_threads--; + _Py_atomic_add_ssize(&tstate->interp->sys_profiling_threads, -1); tstate->c_profilefunc = NULL; } if (tstate->c_tracefunc != NULL) { - tstate->interp->sys_tracing_threads--; + _Py_atomic_add_ssize(&tstate->interp->sys_tracing_threads, -1); tstate->c_tracefunc = NULL; } Py_CLEAR(tstate->c_profileobj); diff --git a/Python/sysmodule.c b/Python/sysmodule.c index 4b7f0cc68daadb..85f6d622541020 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -1206,9 +1206,10 @@ sys__settraceallthreads(PyObject *module, PyObject *arg) argument = arg; } - - PyEval_SetTraceAllThreads(func, argument); - + PyInterpreterState *interp = _PyInterpreterState_GET(); + if (_PyEval_SetTraceAllThreads(interp, func, argument) < 0) { + return NULL; + } Py_RETURN_NONE; } @@ -1286,8 +1287,10 @@ sys__setprofileallthreads(PyObject *module, PyObject *arg) argument = arg; } - PyEval_SetProfileAllThreads(func, argument); - + PyInterpreterState *interp = _PyInterpreterState_GET(); + if (_PyEval_SetProfileAllThreads(interp, func, argument) < 0) { + return NULL; + } Py_RETURN_NONE; } From 400617f8bafffb65d29971a7d23b0767713e66ce Mon Sep 17 00:00:00 2001 From: Sam Gross Date: Wed, 13 Aug 2025 15:28:21 -0400 Subject: [PATCH 2/2] Regen generated files --- Python/generated_cases.c.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index be00bf6eb6a39e..92b67cd5442893 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -5137,7 +5137,15 @@ _Py_CODEUNIT *this_instr = next_instr - 1; (void)this_instr; assert(frame == tstate->current_frame); - if (tstate->tracing == 0) { + #ifdef Py_GIL_DISABLED + // For thread-safety, we need to check instrumentation version + // even when tracing. Otherwise, another thread may concurrently + // re-write the bytecode while we are executing this function. + int check_instrumentation = 1; + #else + int check_instrumentation = (tstate->tracing == 0); + #endif + if (check_instrumentation) { uintptr_t global_version = _Py_atomic_load_uintptr_relaxed(&tstate->eval_breaker) & ~_PY_EVAL_EVENTS_MASK;