gh-115832: Fix instrumentation version mismatch during interpreter shutdown

Include/internal/pycore_code.h

@@ -516,7 +516,7 @@ adaptive_counter_backoff(uint16_t counter) {
 
 #define COMPARISON_NOT_EQUALS (COMPARISON_UNORDERED | COMPARISON_LESS_THAN | COMPARISON_GREATER_THAN)
 
-extern int _Py_Instrument(PyCodeObject *co, PyInterpreterState *interp);
+extern int _Py_Instrument(PyCodeObject *co, PyThreadState *tstate);
 
 extern int _Py_GetBaseOpcode(PyCodeObject *code, int offset);
 

Lib/test/_test_monitoring_shutdown.py

@@ -0,0 +1,30 @@
+#!/usr/bin/env python3
+
+# gh-115832: An object destructor running during the final GC of interpreter
+# shutdown triggered an infinite loop in the instrumentation code.
+
+import sys
+
+class CallableCycle:
+    def __init__(self):
+        self._cycle = self
+
+    def __del__(self):
+        pass
+
+    def __call__(self, code, instruction_offset):
+        pass
+
+def tracefunc(frame, event, arg):
+    pass
+
+def main():
+    tool_id = sys.monitoring.PROFILER_ID
+    event_id = sys.monitoring.events.PY_START
+
+    sys.monitoring.use_tool_id(tool_id, "test profiler")
+    sys.monitoring.set_events(tool_id, event_id)
+    sys.monitoring.register_callback(tool_id, event_id, CallableCycle())
+
+if __name__ == "__main__":
+    sys.exit(main())

Lib/test/test_monitoring.py

@@ -9,6 +9,8 @@
 import types
 import unittest
 import asyncio
+from test import support
+from test.support import script_helper
 
 PAIR = (0,1)
 
@@ -1853,3 +1855,12 @@ def test_func(recorder):
             sys.monitoring.register_callback(TEST_TOOL, E.LINE, None)
             sys.monitoring.set_events(TEST_TOOL, 0)
         self.assertGreater(len(events), 250)
+
+class TestMonitoringAtShutdown(unittest.TestCase):
+
+    def test_monitoring_live_at_shutdown(self):
+        # gh-115832: An object destructor running during the final GC of
+        # interpreter shutdown triggered an infinite loop in the
+        # instrumentation code.
+        script = support.findfile("_test_monitoring_shutdown.py")
+        script_helper.run_test_script(script)

Python/bytecodes.c

@@ -150,7 +150,7 @@ dummy_func(
             uintptr_t code_version = _PyFrame_GetCode(frame)->_co_instrumentation_version;
             assert((code_version & 255) == 0);
             if (code_version != global_version) {
-                int err = _Py_Instrument(_PyFrame_GetCode(frame), tstate->interp);
+                int err = _Py_Instrument(_PyFrame_GetCode(frame), tstate);
                 ERROR_IF(err, error);
                 next_instr = this_instr;
             }
@@ -177,7 +177,7 @@ dummy_func(
             uintptr_t global_version = _Py_atomic_load_uintptr_relaxed(&tstate->eval_breaker) & ~_PY_EVAL_EVENTS_MASK;
             uintptr_t code_version = _PyFrame_GetCode(frame)->_co_instrumentation_version;
             if (code_version != global_version) {
-                if (_Py_Instrument(_PyFrame_GetCode(frame), tstate->interp)) {
+                if (_Py_Instrument(_PyFrame_GetCode(frame), tstate)) {
                     GOTO_ERROR(error);
                 }
                 next_instr = this_instr;

Python/ceval.c

@@ -736,7 +736,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
         }
         /* Because this avoids the RESUME,
          * we need to update instrumentation */
-        _Py_Instrument(_PyFrame_GetCode(frame), tstate->interp);
+        _Py_Instrument(_PyFrame_GetCode(frame), tstate);
         monitor_throw(tstate, frame, frame->instr_ptr);
         /* TO DO -- Monitor throw entry. */
         goto resume_with_error;

Python/instrumentation.c

@@ -889,10 +889,10 @@ static inline int most_significant_bit(uint8_t bits) {
 }
 
 static uint32_t
-global_version(PyInterpreterState *interp)
+global_version(PyThreadState *tstate)
 {
-    return (uint32_t)_Py_atomic_load_uintptr_relaxed(
-        &interp->ceval.instrumentation_version);
+    return (uint32_t)(_Py_atomic_load_uintptr_relaxed(&tstate->eval_breaker) &
+                      ~_PY_EVAL_EVENTS_MASK);
 }
 
 /* Atomically set the given version in the given location, without touching
@@ -931,9 +931,9 @@ set_global_version(PyThreadState *tstate, uint32_t version)
 }
 
 static bool
-is_version_up_to_date(PyCodeObject *code, PyInterpreterState *interp)
+is_version_up_to_date(PyCodeObject *code, PyThreadState *tstate)
 {
-    return global_version(interp) == code->_co_instrumentation_version;
+    return global_version(tstate) == code->_co_instrumentation_version;
 }
 
 #ifndef NDEBUG
@@ -948,8 +948,9 @@ instrumentation_cross_checks(PyInterpreterState *interp, PyCodeObject *code)
 #endif
 
 static inline uint8_t
-get_tools_for_instruction(PyCodeObject *code, PyInterpreterState *interp, int i, int event)
+get_tools_for_instruction(PyCodeObject *code, PyThreadState *tstate, int i, int event)
 {
+    PyInterpreterState *interp = tstate->interp;
     uint8_t tools;
     assert(event != PY_MONITORING_EVENT_LINE);
     assert(event != PY_MONITORING_EVENT_INSTRUCTION);
@@ -959,7 +960,7 @@ get_tools_for_instruction(PyCodeObject *code, PyInterpreterState *interp, int i,
         event = PY_MONITORING_EVENT_CALL;
     }
     if (PY_MONITORING_IS_INSTRUMENTED_EVENT(event)) {
-        CHECK(is_version_up_to_date(code, interp));
+        CHECK(is_version_up_to_date(code, tstate));
         CHECK(instrumentation_cross_checks(interp, code));
         if (code->_co_monitoring->tools) {
             tools = code->_co_monitoring->tools[i];
@@ -1018,7 +1019,7 @@ call_instrumentation_vector(
     assert(args[2] == NULL);
     args[2] = offset_obj;
     PyInterpreterState *interp = tstate->interp;
-    uint8_t tools = get_tools_for_instruction(code, interp, offset, event);
+    uint8_t tools = get_tools_for_instruction(code, tstate, offset, event);
     Py_ssize_t nargsf = nargs | PY_VECTORCALL_ARGUMENTS_OFFSET;
     PyObject **callargs = &args[1];
     int err = 0;
@@ -1156,7 +1157,7 @@ int
 _Py_call_instrumentation_line(PyThreadState *tstate, _PyInterpreterFrame* frame, _Py_CODEUNIT *instr, _Py_CODEUNIT *prev)
 {
     PyCodeObject *code = _PyFrame_GetCode(frame);
-    assert(is_version_up_to_date(code, tstate->interp));
+    assert(is_version_up_to_date(code, tstate));
     assert(instrumentation_cross_checks(tstate->interp, code));
     int i = (int)(instr - _PyCode_CODE(code));
 
@@ -1258,7 +1259,7 @@ int
 _Py_call_instrumentation_instruction(PyThreadState *tstate, _PyInterpreterFrame* frame, _Py_CODEUNIT *instr)
 {
     PyCodeObject *code = _PyFrame_GetCode(frame);
-    assert(is_version_up_to_date(code, tstate->interp));
+    assert(is_version_up_to_date(code, tstate));
     assert(instrumentation_cross_checks(tstate->interp, code));
     int offset = (int)(instr - _PyCode_CODE(code));
     _PyCoMonitoringData *instrumentation_data = code->_co_monitoring;
@@ -1587,11 +1588,12 @@ update_instrumentation_data(PyCodeObject *code, PyInterpreterState *interp)
 }
 
 int
-_Py_Instrument(PyCodeObject *code, PyInterpreterState *interp)
+_Py_Instrument(PyCodeObject *code, PyThreadState *tstate)
 {
-    if (is_version_up_to_date(code, interp)) {
+    PyInterpreterState *interp = tstate->interp;
+    if (is_version_up_to_date(code, tstate)) {
         assert(
-            interp->ceval.instrumentation_version == 0 ||
+            global_version(tstate) == 0 ||
             instrumentation_cross_checks(interp, code)
         );
         return 0;
@@ -1628,7 +1630,7 @@ _Py_Instrument(PyCodeObject *code, PyInterpreterState *interp)
         assert(monitors_are_empty(monitors_and(new_events, removed_events)));
     }
     code->_co_monitoring->active_monitors = active_events;
-    code->_co_instrumentation_version = global_version(interp);
+    code->_co_instrumentation_version = global_version(tstate);
     if (monitors_are_empty(new_events) && monitors_are_empty(removed_events)) {
 #ifdef INSTRUMENT_DEBUG
         sanity_check_instrumentation(code);
@@ -1737,16 +1739,18 @@ _Py_Instrument(PyCodeObject *code, PyInterpreterState *interp)
 
 
 static int
-instrument_all_executing_code_objects(PyInterpreterState *interp) {
+instrument_all_executing_code_objects(PyThreadState *this_tstate) {
     _PyRuntimeState *runtime = &_PyRuntime;
     HEAD_LOCK(runtime);
-    PyThreadState* ts = PyInterpreterState_ThreadHead(interp);
+    PyThreadState* ts = PyInterpreterState_ThreadHead(this_tstate->interp);
     HEAD_UNLOCK(runtime);
     while (ts) {
         _PyInterpreterFrame *frame = ts->current_frame;
         while (frame) {
             if (frame->owner != FRAME_OWNED_BY_CSTACK) {
-                if (_Py_Instrument(_PyFrame_GetCode(frame), interp)) {
+                // _Py_Instrument() takes the current tstate, regardless of
+                // which thread we fetched this code object from.
+                if (_Py_Instrument(_PyFrame_GetCode(frame), this_tstate)) {
                     return -1;
                 }
             }
@@ -1814,21 +1818,22 @@ _PyMonitoring_SetEvents(int tool_id, _PyMonitoringEventSet events)
         return 0;
     }
     set_events(&interp->monitors, tool_id, events);
-    uint32_t new_version = global_version(interp) + MONITORING_VERSION_INCREMENT;
+    uint32_t new_version = global_version(tstate) + MONITORING_VERSION_INCREMENT;
     if (new_version == 0) {
         PyErr_Format(PyExc_OverflowError, "events set too many times");
         return -1;
     }
     set_global_version(tstate, new_version);
     _Py_Executors_InvalidateAll(interp);
-    return instrument_all_executing_code_objects(interp);
+    return instrument_all_executing_code_objects(tstate);
 }
 
 int
 _PyMonitoring_SetLocalEvents(PyCodeObject *code, int tool_id, _PyMonitoringEventSet events)
 {
     assert(0 <= tool_id && tool_id < PY_MONITORING_TOOL_IDS);
-    PyInterpreterState *interp = _PyInterpreterState_GET();
+    PyThreadState *tstate = _PyThreadState_GET();
+    PyInterpreterState *interp = tstate->interp;
     assert(events < (1 << _PY_MONITORING_LOCAL_EVENTS));
     if (code->_co_firsttraceable >= Py_SIZE(code)) {
         PyErr_Format(PyExc_SystemError, "cannot instrument shim code object '%U'", code->co_name);
@@ -1846,12 +1851,12 @@ _PyMonitoring_SetLocalEvents(PyCodeObject *code, int tool_id, _PyMonitoringEvent
         return 0;
     }
     set_local_events(local, tool_id, events);
-    if (is_version_up_to_date(code, interp)) {
+    if (is_version_up_to_date(code, tstate)) {
         /* Force instrumentation update */
         code->_co_instrumentation_version -= MONITORING_VERSION_INCREMENT;
     }
     _Py_Executors_InvalidateDependency(interp, code);
-    if (_Py_Instrument(code, interp)) {
+    if (_Py_Instrument(code, tstate)) {
         return -1;
     }
     return 0;
@@ -2150,15 +2155,15 @@ monitoring_restart_events_impl(PyObject *module)
      */
     PyThreadState *tstate = _PyThreadState_GET();
     PyInterpreterState *interp = tstate->interp;
-    uint32_t restart_version = global_version(interp) + MONITORING_VERSION_INCREMENT;
+    uint32_t restart_version = global_version(tstate) + MONITORING_VERSION_INCREMENT;
     uint32_t new_version = restart_version + MONITORING_VERSION_INCREMENT;
     if (new_version <= MONITORING_VERSION_INCREMENT) {
         PyErr_Format(PyExc_OverflowError, "events set too many times");
         return NULL;
     }
     interp->last_restart_version = restart_version;
     set_global_version(tstate, new_version);
-    if (instrument_all_executing_code_objects(interp)) {
+    if (instrument_all_executing_code_objects(tstate)) {
         return NULL;
     }
     Py_RETURN_NONE;

Python/pystate.c

@@ -795,7 +795,10 @@ interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate)
 
     Py_CLEAR(interp->audit_hooks);
 
+    // At this time, all the threads should be cleared so we don't need atomic
+    // operations for instrumentation_version or eval_breaker.
     interp->ceval.instrumentation_version = 0;
+    tstate->eval_breaker = 0;
 
     for (int i = 0; i < _PY_MONITORING_UNGROUPED_EVENTS; i++) {
         interp->monitors.tools[i] = 0;