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.

Author: limadog9

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)

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()
 

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)
         {