Skip to content

Commit 0adfa84

Browse files
authored
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`.
1 parent 15dc297 commit 0adfa84

File tree

4 files changed

+53
-2
lines changed

4 files changed

+53
-2
lines changed

Lib/test/_test_monitoring_shutdown.py

+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
#!/usr/bin/env python3
2+
3+
# gh-115832: An object destructor running during the final GC of interpreter
4+
# shutdown triggered an infinite loop in the instrumentation code.
5+
6+
import sys
7+
8+
class CallableCycle:
9+
def __init__(self):
10+
self._cycle = self
11+
12+
def __del__(self):
13+
pass
14+
15+
def __call__(self, code, instruction_offset):
16+
pass
17+
18+
def tracefunc(frame, event, arg):
19+
pass
20+
21+
def main():
22+
tool_id = sys.monitoring.PROFILER_ID
23+
event_id = sys.monitoring.events.PY_START
24+
25+
sys.monitoring.use_tool_id(tool_id, "test profiler")
26+
sys.monitoring.set_events(tool_id, event_id)
27+
sys.monitoring.register_callback(tool_id, event_id, CallableCycle())
28+
29+
if __name__ == "__main__":
30+
sys.exit(main())

Lib/test/test_monitoring.py

+11-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@
99
import types
1010
import unittest
1111
import asyncio
12-
from test.support import requires_specialization
12+
from test import support
13+
from test.support import requires_specialization, script_helper
1314

1415
PAIR = (0,1)
1516

@@ -1858,3 +1859,12 @@ def test_func(recorder):
18581859
sys.monitoring.register_callback(TEST_TOOL, E.LINE, None)
18591860
sys.monitoring.set_events(TEST_TOOL, 0)
18601861
self.assertGreater(len(events), 250)
1862+
1863+
class TestMonitoringAtShutdown(unittest.TestCase):
1864+
1865+
def test_monitoring_live_at_shutdown(self):
1866+
# gh-115832: An object destructor running during the final GC of
1867+
# interpreter shutdown triggered an infinite loop in the
1868+
# instrumentation code.
1869+
script = support.findfile("_test_monitoring_shutdown.py")
1870+
script_helper.run_test_script(script)

Python/instrumentation.c

+9-1
Original file line numberDiff line numberDiff line change
@@ -891,8 +891,16 @@ static inline int most_significant_bit(uint8_t bits) {
891891
static uint32_t
892892
global_version(PyInterpreterState *interp)
893893
{
894-
return (uint32_t)_Py_atomic_load_uintptr_relaxed(
894+
uint32_t version = (uint32_t)_Py_atomic_load_uintptr_relaxed(
895895
&interp->ceval.instrumentation_version);
896+
#ifdef Py_DEBUG
897+
PyThreadState *tstate = _PyThreadState_GET();
898+
uint32_t thread_version =
899+
(uint32_t)(_Py_atomic_load_uintptr_relaxed(&tstate->eval_breaker) &
900+
~_PY_EVAL_EVENTS_MASK);
901+
assert(thread_version == version);
902+
#endif
903+
return version;
896904
}
897905

898906
/* Atomically set the given version in the given location, without touching

Python/pystate.c

+3
Original file line numberDiff line numberDiff line change
@@ -795,7 +795,10 @@ interpreter_clear(PyInterpreterState *interp, PyThreadState *tstate)
795795

796796
Py_CLEAR(interp->audit_hooks);
797797

798+
// At this time, all the threads should be cleared so we don't need atomic
799+
// operations for instrumentation_version or eval_breaker.
798800
interp->ceval.instrumentation_version = 0;
801+
tstate->eval_breaker = 0;
799802

800803
for (int i = 0; i < _PY_MONITORING_UNGROUPED_EVENTS; i++) {
801804
interp->monitors.tools[i] = 0;

0 commit comments

Comments
 (0)