Skip to content

Commit a436cd2

Browse files
[3.14] gh-158121: Preserve instruction events when line monitoring is… (#159741)
* [3.14] gh-158121: Preserve instruction events when line monitoring is disabled (GH-158456) (cherry picked from commit f0f76de) Co-authored-by: limadog9 <jakewallin42@gmail.com>
1 parent b97f5e7 commit a436cd2

4 files changed

Lines changed: 146 additions & 3 deletions

File tree

‎Lib/test/test_free_threading/test_monitoring.py‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""Tests monitoring, sys.settrace, and sys.setprofile in a multi-threaded
22
environment to verify things are thread-safe in a free-threaded build"""
33

4+
import dis
45
import sys
56
import threading
67
import time
@@ -251,6 +252,62 @@ def append(self, trace):
251252

252253
@threading_helper.requires_working_threading()
253254
class MonitoringMisc(MonitoringTestMixin, TestCase):
255+
def test_disable_line_keeps_instruction_events(self):
256+
def func(x):
257+
a = x + 1
258+
b = a * 2
259+
return b
260+
261+
code = func.__code__
262+
expected = [instr.offset for instr in dis.get_instructions(func)
263+
if instr.opname != "RESUME"]
264+
# Two threads, each with two lists of instruction offsets (one per call).
265+
# Check that disabling LINE preserves INSTRUCTION events in every
266+
# thread-local bytecode copy, including on subsequent calls.
267+
records = [[[], []] for _ in range(2)]
268+
local = threading.local()
269+
ready = Barrier(len(records) + 1)
270+
start = threading.Event()
271+
272+
def worker(calls):
273+
# Create each thread's bytecode copy before enabling monitoring.
274+
func(1)
275+
ready.wait()
276+
start.wait()
277+
for instructions in calls:
278+
local.instructions = instructions
279+
func(1)
280+
281+
def instruction(code, offset):
282+
local.instructions.append(offset)
283+
284+
def line(code, lineno):
285+
return monitoring.DISABLE
286+
287+
threads = [Thread(target=worker, args=(calls,)) for calls in records]
288+
try:
289+
with threading_helper.start_threads(threads, unlock=start.set):
290+
ready.wait()
291+
monitoring.register_callback(
292+
self.tool_id, monitoring.events.INSTRUCTION, instruction)
293+
monitoring.register_callback(
294+
self.tool_id, monitoring.events.LINE, line)
295+
monitoring.set_local_events(
296+
self.tool_id, code,
297+
monitoring.events.INSTRUCTION | monitoring.events.LINE)
298+
start.set()
299+
finally:
300+
monitoring.set_local_events(self.tool_id, code, 0)
301+
monitoring.register_callback(
302+
self.tool_id, monitoring.events.INSTRUCTION, None)
303+
monitoring.register_callback(
304+
self.tool_id, monitoring.events.LINE, None)
305+
monitoring.restart_events()
306+
307+
for calls in records:
308+
for instructions in calls:
309+
self.assertEqual(instructions, expected)
310+
254311
def register_callback(self, barrier):
255312
barrier.wait()
256313

‎Lib/test/test_monitoring.py‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1207,6 +1207,80 @@ def __call__(self, code, offset):
12071207
class TestLineAndInstructionEvents(CheckEvents):
12081208
maxDiff = None
12091209

1210+
def test_disable_line_keeps_instruction_events(self):
1211+
for local in (False, True):
1212+
# Use the same tool or different tools for LINE and INSTRUCTION.
1213+
for line_tool, instr_tool in (
1214+
(TEST_TOOL, TEST_TOOL),
1215+
(TEST_TOOL2, TEST_TOOL),
1216+
):
1217+
with self.subTest(local=local, line_tool=line_tool,
1218+
instr_tool=instr_tool):
1219+
self.check_disable_line_keeps_instruction_events(
1220+
local, line_tool, instr_tool)
1221+
1222+
def check_disable_line_keeps_instruction_events(self, local, line_tool,
1223+
instr_tool):
1224+
def func(x):
1225+
a = x + 1
1226+
# Split the assignment so line events also move backwards from
1227+
# the expression's line to the assignment's line.
1228+
b = (
1229+
a * 2
1230+
)
1231+
return b
1232+
1233+
code = func.__code__
1234+
instructions = []
1235+
lines = []
1236+
disable = False
1237+
1238+
def instruction(code_arg, offset):
1239+
if code_arg is code:
1240+
instructions.append(offset)
1241+
1242+
def line(code_arg, lineno):
1243+
if code_arg is code:
1244+
lines.append(lineno)
1245+
if disable:
1246+
return sys.monitoring.DISABLE
1247+
1248+
sys.monitoring.register_callback(instr_tool, E.INSTRUCTION, instruction)
1249+
sys.monitoring.register_callback(line_tool, E.LINE, line)
1250+
events = {instr_tool: E.INSTRUCTION}
1251+
events[line_tool] = events.get(line_tool, 0) | E.LINE
1252+
try:
1253+
for tool, mask in events.items():
1254+
if local:
1255+
sys.monitoring.set_local_events(tool, code, mask)
1256+
else:
1257+
sys.monitoring.set_events(tool, mask)
1258+
func(1)
1259+
expected_instructions = instructions[:]
1260+
expected_lines = lines[:]
1261+
self.assertEqual(expected_instructions, [
1262+
inst.offset for inst in dis.get_instructions(func)
1263+
if inst.opname != "RESUME"
1264+
])
1265+
self.assertTrue(expected_lines)
1266+
disable = True
1267+
for restart in (False, True):
1268+
if restart:
1269+
sys.monitoring.restart_events()
1270+
for call in range(2):
1271+
instructions.clear()
1272+
lines.clear()
1273+
func(1)
1274+
self.assertEqual(instructions, expected_instructions)
1275+
self.assertEqual(lines, expected_lines if call == 0 else [])
1276+
finally:
1277+
for tool in events:
1278+
sys.monitoring.set_local_events(tool, code, 0)
1279+
sys.monitoring.set_events(tool, 0)
1280+
sys.monitoring.register_callback(instr_tool, E.INSTRUCTION, None)
1281+
sys.monitoring.register_callback(line_tool, E.LINE, None)
1282+
sys.monitoring.restart_events()
1283+
12101284
def test_simple(self):
12111285

12121286
def func1():
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix missing :mod:`sys.monitoring` ``INSTRUCTION`` events when a ``LINE``
2+
callback returns :data:`sys.monitoring.DISABLE`.

‎Python/instrumentation.c‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -714,9 +714,6 @@ de_instrument_line(PyCodeObject *code, _Py_CODEUNIT *bytecode, _PyCoMonitoringDa
714714
}
715715
_PyCoLineInstrumentationData *lines = monitoring->lines;
716716
int original_opcode = get_original_opcode(lines, i);
717-
if (original_opcode == INSTRUMENTED_INSTRUCTION) {
718-
set_original_opcode(lines, i, monitoring->per_instruction_opcodes[i]);
719-
}
720717
CHECK(original_opcode != 0);
721718
CHECK(original_opcode == _PyOpcode_Deopt[original_opcode]);
722719
FT_ATOMIC_STORE_UINT8(instr->op.code, original_opcode);
@@ -881,7 +878,15 @@ remove_line_tools(PyCodeObject * code, int offset, int tools)
881878
should_de_instrument = ((single_tool & tools) == single_tool);
882879
}
883880
if (should_de_instrument) {
881+
/* Restore all thread-local bytecodes before updating the shared
882+
* original opcode. */
884883
MODIFY_BYTECODE(code, de_instrument_line, monitoring, offset);
884+
if (get_original_opcode(monitoring->lines, offset) ==
885+
INSTRUMENTED_INSTRUCTION)
886+
{
887+
set_original_opcode(monitoring->lines, offset,
888+
monitoring->per_instruction_opcodes[offset]);
889+
}
885890
}
886891
}
887892

@@ -1395,6 +1400,11 @@ _Py_call_instrumentation_line(PyThreadState *tstate, _PyInterpreterFrame* frame,
13951400
uint8_t original_opcode;
13961401
done:
13971402
original_opcode = get_original_opcode(line_data, i);
1403+
if (instr->op.code == INSTRUMENTED_INSTRUCTION) {
1404+
/* A LINE callback may have disabled the last LINE tool while
1405+
* leaving INSTRUCTION monitoring enabled. */
1406+
original_opcode = INSTRUMENTED_INSTRUCTION;
1407+
}
13981408
assert(original_opcode != 0);
13991409
assert(original_opcode != INSTRUMENTED_LINE);
14001410
assert(_PyOpcode_Deopt[original_opcode] == original_opcode);

0 commit comments

Comments
 (0)