Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-115832: Fix instrumentation version mismatch during interpreter shutdown #115856

Merged
merged 5 commits into from
Mar 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions Lib/test/_test_monitoring_shutdown.py
Original file line number Diff line number Diff line change
@@ -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())
12 changes: 11 additions & 1 deletion Lib/test/test_monitoring.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)
10 changes: 9 additions & 1 deletion Python/instrumentation.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions Python/pystate.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading