summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMiss Islington (bot) <31488909+miss-islington@users.noreply.github.com>2025-07-15 15:10:37 (GMT)
committerGitHub <noreply@github.com>2025-07-15 15:10:37 (GMT)
commit9ae74e94a06865aee96827838ce16c8924cbe86f (patch)
tree71932366c8f66e7d486570a471fd147d48cbf09b
parentc939963b53600062f0978c99559e22ab39135402 (diff)
downloadcpython-9ae74e94a06865aee96827838ce16c8924cbe86f.zip
cpython-9ae74e94a06865aee96827838ce16c8924cbe86f.tar.gz
cpython-9ae74e94a06865aee96827838ce16c8924cbe86f.tar.bz2
[3.14] gh-136396: Include instrumentation when creating new copies of the bytecode (GH-136525) (GH-136657)
Previously, we assumed that instrumentation would happen for all copies of the bytecode if the instrumentation version on the code object didn't match the per-interpreter instrumentation version. That assumption was incorrect: instrumentation will exit early if there are no new "events," even if there is an instrumentation version mismatch. To fix this, include the instrumented opcodes when creating new copies of the bytecode, rather than replacing them with their uninstrumented variants. I don't think we have to worry about races between instrumentation and creating new copies of the bytecode: instrumentation and new bytecode creation cannot happen concurrently. Instrumentation requires that either the world is stopped or the code object's per-object lock is held and new bytecode creation requires holding the code object's per-object lock. (cherry picked from commit d995922198304a6de19ac1bec3e36d1e886d8468) Co-authored-by: mpage <mpage@meta.com> Co-authored-by: Kumar Aditya <kumaraditya@python.org>
-rw-r--r--Lib/test/test_free_threading/test_monitoring.py141
-rw-r--r--Misc/NEWS.d/next/Core_and_Builtins/2025-07-10-15-53-16.gh-issue-136525.xAko0e.rst2
-rw-r--r--Objects/codeobject.c19
3 files changed, 161 insertions, 1 deletions
diff --git a/Lib/test/test_free_threading/test_monitoring.py b/Lib/test/test_free_threading/test_monitoring.py
index a480e39..c3d0a2b 100644
--- a/Lib/test/test_free_threading/test_monitoring.py
+++ b/Lib/test/test_free_threading/test_monitoring.py
@@ -2,10 +2,12 @@
environment to verify things are thread-safe in a free-threaded build"""
import sys
+import threading
import time
import unittest
import weakref
+from contextlib import contextmanager
from sys import monitoring
from test.support import threading_helper
from threading import Thread, _PyRLock, Barrier
@@ -192,6 +194,16 @@ class SetProfileMultiThreaded(InstrumentationMultiThreadedMixin, TestCase):
self.set = not self.set
+class TraceBuf:
+ def __init__(self):
+ self.traces = []
+ self.traces_lock = threading.Lock()
+
+ def append(self, trace):
+ with self.traces_lock:
+ self.traces.append(trace)
+
+
@threading_helper.requires_working_threading()
class MonitoringMisc(MonitoringTestMixin, TestCase):
def register_callback(self, barrier):
@@ -246,6 +258,135 @@ class MonitoringMisc(MonitoringTestMixin, TestCase):
finally:
sys.settrace(None)
+ def test_toggle_setprofile_no_new_events(self):
+ # gh-136396: Make sure that profile functions are called for newly
+ # created threads when profiling is toggled but the set of monitoring
+ # events doesn't change
+ traces = []
+
+ def profiler(frame, event, arg):
+ traces.append((frame.f_code.co_name, event, arg))
+
+ def a(x, y):
+ return b(x, y)
+
+ def b(x, y):
+ return max(x, y)
+
+ sys.setprofile(profiler)
+ try:
+ a(1, 2)
+ finally:
+ sys.setprofile(None)
+ traces.clear()
+
+ def thread_main(x, y):
+ sys.setprofile(profiler)
+ try:
+ a(x, y)
+ finally:
+ sys.setprofile(None)
+ t = Thread(target=thread_main, args=(100, 200))
+ t.start()
+ t.join()
+
+ expected = [
+ ("a", "call", None),
+ ("b", "call", None),
+ ("b", "c_call", max),
+ ("b", "c_return", max),
+ ("b", "return", 200),
+ ("a", "return", 200),
+ ("thread_main", "c_call", sys.setprofile),
+ ]
+ self.assertEqual(traces, expected)
+
+ def observe_threads(self, observer, buf):
+ def in_child(ident):
+ return ident
+
+ def child(ident):
+ with observer():
+ in_child(ident)
+
+ def in_parent(ident):
+ return ident
+
+ def parent(barrier, ident):
+ barrier.wait()
+ with observer():
+ t = Thread(target=child, args=(ident,))
+ t.start()
+ t.join()
+ in_parent(ident)
+
+ num_threads = 5
+ barrier = Barrier(num_threads)
+ threads = []
+ for i in range(num_threads):
+ t = Thread(target=parent, args=(barrier, i))
+ t.start()
+ threads.append(t)
+ for t in threads:
+ t.join()
+
+ for i in range(num_threads):
+ self.assertIn(("in_parent", "return", i), buf.traces)
+ self.assertIn(("in_child", "return", i), buf.traces)
+
+ def test_profile_threads(self):
+ buf = TraceBuf()
+
+ def profiler(frame, event, arg):
+ buf.append((frame.f_code.co_name, event, arg))
+
+ @contextmanager
+ def profile():
+ sys.setprofile(profiler)
+ try:
+ yield
+ finally:
+ sys.setprofile(None)
+
+ self.observe_threads(profile, buf)
+
+ def test_trace_threads(self):
+ buf = TraceBuf()
+
+ def tracer(frame, event, arg):
+ buf.append((frame.f_code.co_name, event, arg))
+ return tracer
+
+ @contextmanager
+ def trace():
+ sys.settrace(tracer)
+ try:
+ yield
+ finally:
+ sys.settrace(None)
+
+ self.observe_threads(trace, buf)
+
+ def test_monitor_threads(self):
+ buf = TraceBuf()
+
+ def monitor_py_return(code, off, retval):
+ buf.append((code.co_name, "return", retval))
+
+ monitoring.register_callback(
+ self.tool_id, monitoring.events.PY_RETURN, monitor_py_return
+ )
+
+ monitoring.set_events(
+ self.tool_id, monitoring.events.PY_RETURN
+ )
+
+ @contextmanager
+ def noop():
+ yield
+
+ self.observe_threads(noop, buf)
+
if __name__ == "__main__":
unittest.main()
diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-07-10-15-53-16.gh-issue-136525.xAko0e.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-07-10-15-53-16.gh-issue-136525.xAko0e.rst
new file mode 100644
index 0000000..f28eb2c
--- /dev/null
+++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-07-10-15-53-16.gh-issue-136525.xAko0e.rst
@@ -0,0 +1,2 @@
+Fix issue where per-thread bytecode was not instrumented for newly created
+threads.
diff --git a/Objects/codeobject.c b/Objects/codeobject.c
index ba178abc..42e0216 100644
--- a/Objects/codeobject.c
+++ b/Objects/codeobject.c
@@ -3330,12 +3330,29 @@ _PyCodeArray_New(Py_ssize_t size)
return arr;
}
+// Get the underlying code unit, leaving instrumentation
+static _Py_CODEUNIT
+deopt_code_unit(PyCodeObject *code, int i)
+{
+ _Py_CODEUNIT *src_instr = _PyCode_CODE(code) + i;
+ _Py_CODEUNIT inst = {
+ .cache = FT_ATOMIC_LOAD_UINT16_RELAXED(*(uint16_t *)src_instr)};
+ int opcode = inst.op.code;
+ if (opcode < MIN_INSTRUMENTED_OPCODE) {
+ inst.op.code = _PyOpcode_Deopt[opcode];
+ assert(inst.op.code < MIN_SPECIALIZED_OPCODE);
+ }
+ // JIT should not be enabled with free-threading
+ assert(inst.op.code != ENTER_EXECUTOR);
+ return inst;
+}
+
static void
copy_code(_Py_CODEUNIT *dst, PyCodeObject *co)
{
int code_len = (int) Py_SIZE(co);
for (int i = 0; i < code_len; i += _PyInstruction_GetLength(co, i)) {
- dst[i] = _Py_GetBaseCodeUnit(co, i);
+ dst[i] = deopt_code_unit(co, i);
}
_PyCode_Quicken(dst, code_len, 1);
}