Skip to content

Commit 64e7708

Browse files
[3.15] gh-158121: Preserve instruction events when line monitoring is disabled (GH-158456) (#159733)
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 b00e8fd commit 64e7708

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
@@ -1192,6 +1192,80 @@ def __call__(self, code, offset):
11921192
class TestLineAndInstructionEvents(CheckEvents):
11931193
maxDiff = None
11941194

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

11971271
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
@@ -715,9 +715,6 @@ de_instrument_line(PyCodeObject *code, _Py_CODEUNIT *bytecode, _PyCoMonitoringDa
715715
}
716716
_PyCoLineInstrumentationData *lines = monitoring->lines;
717717
int original_opcode = _PyCode_GetOriginalOpcode(lines, i);
718-
if (original_opcode == INSTRUMENTED_INSTRUCTION) {
719-
set_original_opcode(lines, i, monitoring->per_instruction_opcodes[i]);
720-
}
721718
CHECK(original_opcode != 0);
722719
CHECK(original_opcode == _PyOpcode_Deopt[original_opcode]);
723720
FT_ATOMIC_STORE_UINT8(instr->op.code, original_opcode);
@@ -882,7 +879,15 @@ remove_line_tools(PyCodeObject * code, int offset, int tools)
882879
should_de_instrument = ((single_tool & tools) == single_tool);
883880
}
884881
if (should_de_instrument) {
882+
/* Restore all thread-local bytecodes before updating the shared
883+
* original opcode. */
885884
MODIFY_BYTECODE(code, de_instrument_line, monitoring, offset);
885+
if (_PyCode_GetOriginalOpcode(monitoring->lines, offset) ==
886+
INSTRUMENTED_INSTRUCTION)
887+
{
888+
set_original_opcode(monitoring->lines, offset,
889+
monitoring->per_instruction_opcodes[offset]);
890+
}
886891
}
887892
}
888893

@@ -1424,6 +1429,11 @@ _Py_call_instrumentation_line(PyThreadState *tstate, _PyInterpreterFrame* frame,
14241429
uint8_t original_opcode;
14251430
done:
14261431
original_opcode = _PyCode_GetOriginalOpcode(line_data, i);
1432+
if (instr->op.code == INSTRUMENTED_INSTRUCTION) {
1433+
/* A LINE callback may have disabled the last LINE tool while
1434+
* leaving INSTRUCTION monitoring enabled. */
1435+
original_opcode = INSTRUMENTED_INSTRUCTION;
1436+
}
14271437
assert(original_opcode != 0);
14281438
assert(original_opcode != INSTRUMENTED_LINE);
14291439
assert(_PyOpcode_Deopt[original_opcode] == original_opcode);

0 commit comments

Comments
 (0)