From beb1957dddd8e780923be72fa0ce9896261b6fca Mon Sep 17 00:00:00 2001
From: Brett Simmers <swtaarrs@users.noreply.github.com>
Date: Mon, 4 Mar 2024 08:29:39 -0800
Subject: [PATCH] gh-115832: Fix instrumentation version mismatch during
 interpreter shutdown (#115856)

A previous commit introduced a bug to `interpreter_clear()`: it set
`interp->ceval.instrumentation_version` to 0, without making the corresponding
change to `tstate->eval_breaker` (which holds a thread-local copy of the
version). After this happens, Python code can still run due to object finalizers
during a GC, and the version check in bytecodes.c will see a different result
than the one in instrumentation.c causing an infinite loop.

The fix itself is straightforward: clear `tstate->eval_breaker` when clearing
`interp->ceval.instrumentation_version`.
---
 Lib/test/_test_monitoring_shutdown.py | 30 +++++++++++++++++++++++++++
 Lib/test/test_monitoring.py           | 12 ++++++++++-
 Python/instrumentation.c              | 10 ++++++++-
 Python/pystate.c                      |  3 +++
 4 files changed, 53 insertions(+), 2 deletions(-)
 create mode 100644 Lib/test/_test_monitoring_shutdown.py

diff --git a/Lib/test/_test_monitoring_shutdown.py b/Lib/test/_test_monitoring_shutdown.py
new file mode 100644
index 000000000000000..3d0fbec029dc09c
--- /dev/null
+++ b/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())
diff --git a/Lib/test/test_monitoring.py b/Lib/test/test_monitoring.py
index b02795903f6d175..1e77eb6a2eea4ca 100644
--- a/Lib/test/test_monitoring.py
+++ b/Lib/test/test_monitoring.py
@@ -9,7 +9,8 @@
 import types
 import unittest
 import asyncio
-from test.support import requires_specialization
+from test import support
+from test.support import requires_specialization, script_helper
 
 PAIR = (0,1)
 
@@ -1858,3 +1859,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)
diff --git a/Python/instrumentation.c b/Python/instrumentation.c
index 6f1bc2e0a107df5..018cd662b1561af 100644
--- a/Python/instrumentation.c
+++ b/Python/instrumentation.c
@@ -891,8 +891,16 @@ static inline int most_significant_bit(uint8_t bits) {
 static uint32_t
 global_version(PyInterpreterState *interp)
 {
-    return (uint32_t)_Py_atomic_load_uintptr_relaxed(
+    uint32_t version = (uint32_t)_Py_atomic_load_uintptr_relaxed(
         &interp->ceval.instrumentation_version);
+#ifdef Py_DEBUG
+    PyThreadState *tstate = _PyThreadState_GET();
+    uint32_t thread_version =
+        (uint32_t)(_Py_atomic_load_uintptr_relaxed(&tstate->eval_breaker) &
+                   ~_PY_EVAL_EVENTS_MASK);
+    assert(thread_version == version);
+#endif
+    return version;
 }
 
 /* Atomically set the given version in the given location, without touching
diff --git a/Python/pystate.c b/Python/pystate.c
index a370fff857af852..3d6394f81da816d 100644
--- a/Python/pystate.c
+++ b/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;