From 8254a8a8a8818d36fd98d87fa4fd7c2d58ebc696 Mon Sep 17 00:00:00 2001 From: Jake Wallin Date: Tue, 29 Sep 2026 18:36:32 -0400 Subject: [PATCH 1/2] gh-158121: Preserve instruction events when line monitoring is disabled --- .../test_free_threading/test_monitoring.py | 54 +++++++++++++++ Lib/test/test_monitoring.py | 66 +++++++++++++++++++ ...-09-29-14-35-00.gh-issue-158121.Kj4nQ8.rst | 2 + Python/instrumentation.c | 16 ++++- 4 files changed, 135 insertions(+), 3 deletions(-) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-09-29-14-35-00.gh-issue-158121.Kj4nQ8.rst diff --git a/Lib/test/test_free_threading/test_monitoring.py b/Lib/test/test_free_threading/test_monitoring.py index 2cd6e7b035ecb41..0fc1e62ae292c17 100644 --- a/Lib/test/test_free_threading/test_monitoring.py +++ b/Lib/test/test_free_threading/test_monitoring.py @@ -1,6 +1,7 @@ """Tests monitoring, sys.settrace, and sys.setprofile in a multi-threaded environment to verify things are thread-safe in a free-threaded build""" +import dis import sys import threading import time @@ -251,6 +252,59 @@ def append(self, trace): @threading_helper.requires_working_threading() class MonitoringMisc(MonitoringTestMixin, TestCase): + def test_disable_line_keeps_instruction_events(self): + def func(x): + a = x + 1 + b = a * 2 + return b + + code = func.__code__ + expected = [instr.offset for instr in dis.get_instructions(func) + if instr.opname != "RESUME"] + records = [[[], []] for _ in range(2)] + local = threading.local() + ready = Barrier(len(records) + 1) + start = threading.Event() + + def worker(calls): + # Create each thread's bytecode copy before enabling monitoring. + func(1) + ready.wait() + start.wait() + for instructions in calls: + local.instructions = instructions + func(1) + + def instruction(code, offset): + local.instructions.append(offset) + + def line(code, lineno): + return monitoring.DISABLE + + threads = [Thread(target=worker, args=(calls,)) for calls in records] + try: + with threading_helper.start_threads(threads, unlock=start.set): + ready.wait() + monitoring.register_callback( + self.tool_id, monitoring.events.INSTRUCTION, instruction) + monitoring.register_callback( + self.tool_id, monitoring.events.LINE, line) + monitoring.set_local_events( + self.tool_id, code, + monitoring.events.INSTRUCTION | monitoring.events.LINE) + start.set() + finally: + monitoring.set_local_events(self.tool_id, code, 0) + monitoring.register_callback( + self.tool_id, monitoring.events.INSTRUCTION, None) + monitoring.register_callback( + self.tool_id, monitoring.events.LINE, None) + monitoring.restart_events() + + for calls in records: + for instructions in calls: + self.assertEqual(instructions, expected) + def register_callback(self, barrier): barrier.wait() diff --git a/Lib/test/test_monitoring.py b/Lib/test/test_monitoring.py index 5c2d69934b02ea6..b9921ae0f05d149 100644 --- a/Lib/test/test_monitoring.py +++ b/Lib/test/test_monitoring.py @@ -1192,6 +1192,72 @@ def __call__(self, code, offset): class TestLineAndInstructionEvents(CheckEvents): maxDiff = None + def test_disable_line_keeps_instruction_events(self): + for local in (False, True): + for line_tool in (TEST_TOOL, TEST_TOOL2): + with self.subTest(local=local, line_tool=line_tool): + self.check_disable_line_keeps_instruction_events( + local, line_tool) + + def check_disable_line_keeps_instruction_events(self, local, line_tool): + def func(x): + a = x + 1 + b = ( + a * 2 + ) + return b + + code = func.__code__ + instructions = [] + lines = [] + disable = False + + def instruction(code_arg, offset): + if code_arg is code: + instructions.append(offset) + + def line(code_arg, lineno): + if code_arg is code: + lines.append(lineno) + if disable: + return sys.monitoring.DISABLE + + sys.monitoring.register_callback(TEST_TOOL, E.INSTRUCTION, instruction) + sys.monitoring.register_callback(line_tool, E.LINE, line) + events = {TEST_TOOL: E.INSTRUCTION} + events[line_tool] = events.get(line_tool, 0) | E.LINE + try: + for tool, mask in events.items(): + if local: + sys.monitoring.set_local_events(tool, code, mask) + else: + sys.monitoring.set_events(tool, mask) + func(1) + expected_instructions = instructions[:] + expected_lines = lines[:] + self.assertEqual(expected_instructions, [ + inst.offset for inst in dis.get_instructions(func) + if inst.opname != "RESUME" + ]) + self.assertTrue(expected_lines) + disable = True + for restart in (False, True): + if restart: + sys.monitoring.restart_events() + for call in range(2): + instructions.clear() + lines.clear() + func(1) + self.assertEqual(instructions, expected_instructions) + self.assertEqual(lines, expected_lines if call == 0 else []) + finally: + for tool in events: + sys.monitoring.set_local_events(tool, code, 0) + sys.monitoring.set_events(tool, 0) + sys.monitoring.register_callback(TEST_TOOL, E.INSTRUCTION, None) + sys.monitoring.register_callback(line_tool, E.LINE, None) + sys.monitoring.restart_events() + def test_simple(self): def func1(): diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-09-29-14-35-00.gh-issue-158121.Kj4nQ8.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-09-29-14-35-00.gh-issue-158121.Kj4nQ8.rst new file mode 100644 index 000000000000000..3256ce99a84952b --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-09-29-14-35-00.gh-issue-158121.Kj4nQ8.rst @@ -0,0 +1,2 @@ +Fix missing :mod:`sys.monitoring` ``INSTRUCTION`` events when a ``LINE`` +callback returns :data:`sys.monitoring.DISABLE`. diff --git a/Python/instrumentation.c b/Python/instrumentation.c index 806d3fbf5d6b192..81f6e4232352373 100644 --- a/Python/instrumentation.c +++ b/Python/instrumentation.c @@ -715,9 +715,6 @@ de_instrument_line(PyCodeObject *code, _Py_CODEUNIT *bytecode, _PyCoMonitoringDa } _PyCoLineInstrumentationData *lines = monitoring->lines; int original_opcode = _PyCode_GetOriginalOpcode(lines, i); - if (original_opcode == INSTRUMENTED_INSTRUCTION) { - set_original_opcode(lines, i, monitoring->per_instruction_opcodes[i]); - } CHECK(original_opcode != 0); CHECK(original_opcode == _PyOpcode_Deopt[original_opcode]); FT_ATOMIC_STORE_UINT8(instr->op.code, original_opcode); @@ -883,6 +880,14 @@ remove_line_tools(PyCodeObject * code, int offset, int tools) } if (should_de_instrument) { MODIFY_BYTECODE(code, de_instrument_line, monitoring, offset); + /* Restore all thread-local bytecodes before updating the shared + * original opcode. */ + if (_PyCode_GetOriginalOpcode(monitoring->lines, offset) == + INSTRUMENTED_INSTRUCTION) + { + set_original_opcode(monitoring->lines, offset, + monitoring->per_instruction_opcodes[offset]); + } } } @@ -1424,6 +1429,11 @@ _Py_call_instrumentation_line(PyThreadState *tstate, _PyInterpreterFrame* frame, uint8_t original_opcode; done: original_opcode = _PyCode_GetOriginalOpcode(line_data, i); + if (instr->op.code == INSTRUMENTED_INSTRUCTION) { + /* A LINE callback may have disabled the last LINE tool while + * leaving INSTRUCTION monitoring enabled. */ + original_opcode = INSTRUMENTED_INSTRUCTION; + } assert(original_opcode != 0); assert(original_opcode != INSTRUMENTED_LINE); assert(_PyOpcode_Deopt[original_opcode] == original_opcode); From f7e198ca93923ef67c89e337ecde201253b079b2 Mon Sep 17 00:00:00 2001 From: Jake Wallin Date: Fri, 2 Oct 2026 08:27:31 -0400 Subject: [PATCH 2/2] gh-158121: Clarify monitoring regression tests and opcode restoration Address review feedback by documenting the per-thread call records and multiline assignment, parameterizing both monitoring tool IDs, and placing the restoration comment before MODIFY_BYTECODE. Validation: Windows x64 Debug test_monitoring (99 tests); free-threaded Debug test_monitoring and test_free_threading.test_monitoring (113 tests); Ruff, patchcheck, and git diff --check. --- .../test_free_threading/test_monitoring.py | 3 +++ Lib/test/test_monitoring.py | 22 +++++++++++++------ Python/instrumentation.c | 2 +- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/Lib/test/test_free_threading/test_monitoring.py b/Lib/test/test_free_threading/test_monitoring.py index 0fc1e62ae292c17..1ce84ce38d8273b 100644 --- a/Lib/test/test_free_threading/test_monitoring.py +++ b/Lib/test/test_free_threading/test_monitoring.py @@ -261,6 +261,9 @@ def func(x): code = func.__code__ expected = [instr.offset for instr in dis.get_instructions(func) if instr.opname != "RESUME"] + # Two threads, each with two lists of instruction offsets (one per call). + # Check that disabling LINE preserves INSTRUCTION events in every + # thread-local bytecode copy, including on subsequent calls. records = [[[], []] for _ in range(2)] local = threading.local() ready = Barrier(len(records) + 1) diff --git a/Lib/test/test_monitoring.py b/Lib/test/test_monitoring.py index b9921ae0f05d149..2508d6a5e12f4e6 100644 --- a/Lib/test/test_monitoring.py +++ b/Lib/test/test_monitoring.py @@ -1194,14 +1194,22 @@ class TestLineAndInstructionEvents(CheckEvents): def test_disable_line_keeps_instruction_events(self): for local in (False, True): - for line_tool in (TEST_TOOL, TEST_TOOL2): - with self.subTest(local=local, line_tool=line_tool): + # Use the same tool or different tools for LINE and INSTRUCTION. + for line_tool, instr_tool in ( + (TEST_TOOL, TEST_TOOL), + (TEST_TOOL2, TEST_TOOL), + ): + with self.subTest(local=local, line_tool=line_tool, + instr_tool=instr_tool): self.check_disable_line_keeps_instruction_events( - local, line_tool) + local, line_tool, instr_tool) - def check_disable_line_keeps_instruction_events(self, local, line_tool): + def check_disable_line_keeps_instruction_events(self, local, line_tool, + instr_tool): def func(x): a = x + 1 + # Split the assignment so line events also move backwards from + # the expression's line to the assignment's line. b = ( a * 2 ) @@ -1222,9 +1230,9 @@ def line(code_arg, lineno): if disable: return sys.monitoring.DISABLE - sys.monitoring.register_callback(TEST_TOOL, E.INSTRUCTION, instruction) + sys.monitoring.register_callback(instr_tool, E.INSTRUCTION, instruction) sys.monitoring.register_callback(line_tool, E.LINE, line) - events = {TEST_TOOL: E.INSTRUCTION} + events = {instr_tool: E.INSTRUCTION} events[line_tool] = events.get(line_tool, 0) | E.LINE try: for tool, mask in events.items(): @@ -1254,7 +1262,7 @@ def line(code_arg, lineno): for tool in events: sys.monitoring.set_local_events(tool, code, 0) sys.monitoring.set_events(tool, 0) - sys.monitoring.register_callback(TEST_TOOL, E.INSTRUCTION, None) + sys.monitoring.register_callback(instr_tool, E.INSTRUCTION, None) sys.monitoring.register_callback(line_tool, E.LINE, None) sys.monitoring.restart_events() diff --git a/Python/instrumentation.c b/Python/instrumentation.c index 81f6e4232352373..b8a8383f19c633f 100644 --- a/Python/instrumentation.c +++ b/Python/instrumentation.c @@ -879,9 +879,9 @@ remove_line_tools(PyCodeObject * code, int offset, int tools) should_de_instrument = ((single_tool & tools) == single_tool); } if (should_de_instrument) { - MODIFY_BYTECODE(code, de_instrument_line, monitoring, offset); /* Restore all thread-local bytecodes before updating the shared * original opcode. */ + MODIFY_BYTECODE(code, de_instrument_line, monitoring, offset); if (_PyCode_GetOriginalOpcode(monitoring->lines, offset) == INSTRUMENTED_INSTRUCTION) {