From e2496159b9a4899bb45645a59c2104560268604b Mon Sep 17 00:00:00 2001 From: Tian Gao Date: Tue, 25 Apr 2023 13:53:38 -0700 Subject: [PATCH 1/2] Remove line & instruction instrumentations before adding them back --- Lib/test/test_monitoring.py | 36 ++++++++++++++++++++++++++++++++ Python/instrumentation.c | 41 +++++++++++++++++++++++++++++-------- 2 files changed, 69 insertions(+), 8 deletions(-) diff --git a/Lib/test/test_monitoring.py b/Lib/test/test_monitoring.py index 738ace923cc523..a493bb54d70d38 100644 --- a/Lib/test/test_monitoring.py +++ b/Lib/test/test_monitoring.py @@ -876,6 +876,42 @@ def func3(): ('instruction', 'func3', 34), ('line', 'check_events', 11)]) + def test_with_restart(self): + def func1(): + line1 = 1 + line2 = 2 + line3 = 3 + + self.check_events(func1, recorders = LINE_AND_INSTRUCTION_RECORDERS, expected = [ + ('line', 'check_events', 10), + ('line', 'func1', 1), + ('instruction', 'func1', 2), + ('instruction', 'func1', 4), + ('line', 'func1', 2), + ('instruction', 'func1', 6), + ('instruction', 'func1', 8), + ('line', 'func1', 3), + ('instruction', 'func1', 10), + ('instruction', 'func1', 12), + ('instruction', 'func1', 14), + ('line', 'check_events', 11)]) + + sys.monitoring.restart_events() + + self.check_events(func1, recorders = LINE_AND_INSTRUCTION_RECORDERS, expected = [ + ('line', 'check_events', 10), + ('line', 'func1', 1), + ('instruction', 'func1', 2), + ('instruction', 'func1', 4), + ('line', 'func1', 2), + ('instruction', 'func1', 6), + ('instruction', 'func1', 8), + ('line', 'func1', 3), + ('instruction', 'func1', 10), + ('instruction', 'func1', 12), + ('instruction', 'func1', 14), + ('line', 'check_events', 11)]) + class TestInstallIncrementallly(MonitoringTestBase, unittest.TestCase): def check_events(self, func, must_include, tool=TEST_TOOL, recorders=(ExceptionRecorder,)): diff --git a/Python/instrumentation.c b/Python/instrumentation.c index 8334f596eb3e19..f4a52709414875 100644 --- a/Python/instrumentation.c +++ b/Python/instrumentation.c @@ -1474,25 +1474,25 @@ _Py_Instrument(PyCodeObject *code, PyInterpreterState *interp) } } } - uint8_t new_line_tools = new_events.tools[PY_MONITORING_EVENT_LINE]; + + // GH-103845: We need to remove both the line and instruction instrumentation before + // adding new ones, otherwise we may remove the newly added instrumentation. + uint8_t removed_line_tools = removed_events.tools[PY_MONITORING_EVENT_LINE]; - if (new_line_tools | removed_line_tools) { + uint8_t removed_per_instruction_tools = removed_events.tools[PY_MONITORING_EVENT_INSTRUCTION]; + + if (removed_line_tools) { _PyCoLineInstrumentationData *line_data = code->_co_monitoring->lines; for (int i = code->_co_firsttraceable; i < code_len;) { if (line_data[i].original_opcode) { if (removed_line_tools) { remove_line_tools(code, i, removed_line_tools); } - if (new_line_tools) { - add_line_tools(code, i, new_line_tools); - } } i += instruction_length(code, i); } } - uint8_t new_per_instruction_tools = new_events.tools[PY_MONITORING_EVENT_INSTRUCTION]; - uint8_t removed_per_instruction_tools = removed_events.tools[PY_MONITORING_EVENT_INSTRUCTION]; - if (new_per_instruction_tools | removed_per_instruction_tools) { + if (removed_per_instruction_tools) { for (int i = code->_co_firsttraceable; i < code_len;) { int opcode = _Py_GetBaseOpcode(code, i); if (opcode == RESUME || opcode == END_FOR) { @@ -1502,6 +1502,31 @@ _Py_Instrument(PyCodeObject *code, PyInterpreterState *interp) if (removed_per_instruction_tools) { remove_per_instruction_tools(code, i, removed_per_instruction_tools); } + i += instruction_length(code, i); + } + } + + uint8_t new_line_tools = new_events.tools[PY_MONITORING_EVENT_LINE]; + uint8_t new_per_instruction_tools = new_events.tools[PY_MONITORING_EVENT_INSTRUCTION]; + + if (new_line_tools) { + _PyCoLineInstrumentationData *line_data = code->_co_monitoring->lines; + for (int i = code->_co_firsttraceable; i < code_len;) { + if (line_data[i].original_opcode) { + if (new_line_tools) { + add_line_tools(code, i, new_line_tools); + } + } + i += instruction_length(code, i); + } + } + if (new_per_instruction_tools) { + for (int i = code->_co_firsttraceable; i < code_len;) { + int opcode = _Py_GetBaseOpcode(code, i); + if (opcode == RESUME || opcode == END_FOR) { + i += instruction_length(code, i); + continue; + } if (new_per_instruction_tools) { add_per_instruction_tools(code, i, new_per_instruction_tools); } From 12826e615fbed63c00c823385c8bfb68b08c674c Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Tue, 25 Apr 2023 20:56:03 +0000 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2023-04-25-20-56-01.gh-issue-103845.V7NYFn.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-04-25-20-56-01.gh-issue-103845.V7NYFn.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-04-25-20-56-01.gh-issue-103845.V7NYFn.rst b/Misc/NEWS.d/next/Core and Builtins/2023-04-25-20-56-01.gh-issue-103845.V7NYFn.rst new file mode 100644 index 00000000000000..e8434854cde632 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-04-25-20-56-01.gh-issue-103845.V7NYFn.rst @@ -0,0 +1 @@ +Remove both line and instruction instrumentation before adding new ones for monitoring, to avoid newly added instrumentation being removed immediately.