]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
gh-144446: Fix some frame object thread-safety issues (gh-144479)
authorSam Gross <colesbury@gmail.com>
Fri, 6 Feb 2026 14:43:36 +0000 (09:43 -0500)
committerGitHub <noreply@github.com>
Fri, 6 Feb 2026 14:43:36 +0000 (09:43 -0500)
Fix thread-safety issues when accessing frame attributes while another
thread is executing the frame:

- Add critical section to frame_repr() to prevent races when accessing
  the frame's code object and line number

- Add _Py_NO_SANITIZE_THREAD to PyUnstable_InterpreterFrame_GetLasti()
  to allow intentional racy reads of instr_ptr.

- Fix take_ownership() to not write to the original frame's f_executable

Lib/test/test_free_threading/test_frame.py [new file with mode: 0644]
Misc/NEWS.d/next/Core_and_Builtins/2026-02-03-17-08-13.gh-issue-144446.db5619.rst [new file with mode: 0644]
Objects/frameobject.c
Python/frame.c

diff --git a/Lib/test/test_free_threading/test_frame.py b/Lib/test/test_free_threading/test_frame.py
new file mode 100644 (file)
index 0000000..bea49df
--- /dev/null
@@ -0,0 +1,151 @@
+import functools
+import sys
+import threading
+import unittest
+
+from test.support import threading_helper
+
+threading_helper.requires_working_threading(module=True)
+
+
+def run_with_frame(funcs, runner=None, iters=10):
+    """Run funcs with a frame from another thread that is currently executing.
+
+    Args:
+        funcs: A function or list of functions that take a frame argument
+        runner: Optional function to run in the executor thread. If provided,
+                it will be called and should return eventually. The frame
+                passed to funcs will be the runner's frame.
+        iters: Number of iterations each func should run
+    """
+    if not isinstance(funcs, list):
+        funcs = [funcs]
+
+    frame_var = None
+    e = threading.Event()
+    b = threading.Barrier(len(funcs) + 1)
+
+    if runner is None:
+        def runner():
+            j = 0
+            for i in range(100):
+                j += i
+
+    def executor():
+        nonlocal frame_var
+        frame_var = sys._getframe()
+        e.set()
+        b.wait()
+        runner()
+
+    def func_wrapper(func):
+        e.wait()
+        frame = frame_var
+        b.wait()
+        for _ in range(iters):
+            func(frame)
+
+    test_funcs = [functools.partial(func_wrapper, f) for f in funcs]
+    threading_helper.run_concurrently([executor] + test_funcs)
+
+
+class TestFrameRaces(unittest.TestCase):
+    def test_concurrent_f_lasti(self):
+        run_with_frame(lambda frame: frame.f_lasti)
+
+    def test_concurrent_f_lineno(self):
+        run_with_frame(lambda frame: frame.f_lineno)
+
+    def test_concurrent_f_code(self):
+        run_with_frame(lambda frame: frame.f_code)
+
+    def test_concurrent_f_back(self):
+        run_with_frame(lambda frame: frame.f_back)
+
+    def test_concurrent_f_globals(self):
+        run_with_frame(lambda frame: frame.f_globals)
+
+    def test_concurrent_f_builtins(self):
+        run_with_frame(lambda frame: frame.f_builtins)
+
+    def test_concurrent_f_locals(self):
+        run_with_frame(lambda frame: frame.f_locals)
+
+    def test_concurrent_f_trace_read(self):
+        run_with_frame(lambda frame: frame.f_trace)
+
+    def test_concurrent_f_trace_opcodes_read(self):
+        run_with_frame(lambda frame: frame.f_trace_opcodes)
+
+    def test_concurrent_repr(self):
+        run_with_frame(lambda frame: repr(frame))
+
+    def test_concurrent_f_trace_write(self):
+        def trace_func(frame, event, arg):
+            return trace_func
+
+        def writer(frame):
+            frame.f_trace = trace_func
+            frame.f_trace = None
+
+        run_with_frame(writer)
+
+    def test_concurrent_f_trace_read_write(self):
+        # Test concurrent reads and writes of f_trace on a live frame.
+        def trace_func(frame, event, arg):
+            return trace_func
+
+        def reader(frame):
+            _ = frame.f_trace
+
+        def writer(frame):
+            frame.f_trace = trace_func
+            frame.f_trace = None
+
+        run_with_frame([reader, writer, reader, writer])
+
+    def test_concurrent_f_trace_opcodes_write(self):
+        def writer(frame):
+            frame.f_trace_opcodes = True
+            frame.f_trace_opcodes = False
+
+        run_with_frame(writer)
+
+    def test_concurrent_f_trace_opcodes_read_write(self):
+        # Test concurrent reads and writes of f_trace_opcodes on a live frame.
+        def reader(frame):
+            _ = frame.f_trace_opcodes
+
+        def writer(frame):
+            frame.f_trace_opcodes = True
+            frame.f_trace_opcodes = False
+
+        run_with_frame([reader, writer, reader, writer])
+
+    def test_concurrent_frame_clear(self):
+        # Test race between frame.clear() and attribute reads.
+        def create_frame():
+            x = 1
+            y = 2
+            return sys._getframe()
+
+        frame = create_frame()
+
+        def reader():
+            for _ in range(10):
+                try:
+                    _ = frame.f_locals
+                    _ = frame.f_code
+                    _ = frame.f_lineno
+                except ValueError:
+                    # Frame may be cleared
+                    pass
+
+        def clearer():
+            frame.clear()
+
+        threading_helper.run_concurrently([reader, reader, clearer])
+
+
+if __name__ == "__main__":
+    unittest.main()
diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-02-03-17-08-13.gh-issue-144446.db5619.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-02-03-17-08-13.gh-issue-144446.db5619.rst
new file mode 100644 (file)
index 0000000..71cf493
--- /dev/null
@@ -0,0 +1,2 @@
+Fix data races in the free-threaded build when reading frame object attributes
+while another thread is executing the frame.
index 1d4c0f6785c4b849af9dc30ad8ee6c907448955a..9d774a71edb7977db8349ba5c51e2eabbed6b55b 100644 (file)
@@ -1049,11 +1049,11 @@ static PyObject *
 frame_lasti_get_impl(PyFrameObject *self)
 /*[clinic end generated code: output=03275b4f0327d1a2 input=0225ed49cb1fbeeb]*/
 {
-    int lasti = _PyInterpreterFrame_LASTI(self->f_frame);
+    int lasti = PyUnstable_InterpreterFrame_GetLasti(self->f_frame);
     if (lasti < 0) {
         return PyLong_FromLong(-1);
     }
-    return PyLong_FromLong(lasti * sizeof(_Py_CODEUNIT));
+    return PyLong_FromLong(lasti);
 }
 
 /*[clinic input]
@@ -2053,11 +2053,15 @@ static PyObject *
 frame_repr(PyObject *op)
 {
     PyFrameObject *f = PyFrameObject_CAST(op);
+    PyObject *result;
+    Py_BEGIN_CRITICAL_SECTION(f);
     int lineno = PyFrame_GetLineNumber(f);
     PyCodeObject *code = _PyFrame_GetCode(f->f_frame);
-    return PyUnicode_FromFormat(
+    result = PyUnicode_FromFormat(
         "<frame at %p, file %R, line %d, code %S>",
         f, code->co_filename, lineno, code->co_name);
+    Py_END_CRITICAL_SECTION();
+    return result;
 }
 
 static PyMethodDef frame_methods[] = {
index da8f9037e8287a2dc4d6bb0a5761996a0e3614a4..ff81eb0b3020c788617994cbfc93a5bc2dae68ea 100644 (file)
@@ -54,7 +54,7 @@ take_ownership(PyFrameObject *f, _PyInterpreterFrame *frame)
     _PyFrame_Copy(frame, new_frame);
     // _PyFrame_Copy takes the reference to the executable,
     // so we need to restore it.
-    frame->f_executable = PyStackRef_DUP(new_frame->f_executable);
+    new_frame->f_executable = PyStackRef_DUP(new_frame->f_executable);
     f->f_frame = new_frame;
     new_frame->owner = FRAME_OWNED_BY_FRAME_OBJECT;
     if (_PyFrame_IsIncomplete(new_frame)) {
@@ -135,14 +135,14 @@ PyUnstable_InterpreterFrame_GetCode(struct _PyInterpreterFrame *frame)
     return PyStackRef_AsPyObjectNew(frame->f_executable);
 }
 
-int
+// NOTE: We allow racy accesses to the instruction pointer from other threads
+// for sys._current_frames() and similar APIs.
+int _Py_NO_SANITIZE_THREAD
 PyUnstable_InterpreterFrame_GetLasti(struct _PyInterpreterFrame *frame)
 {
     return _PyInterpreterFrame_LASTI(frame) * sizeof(_Py_CODEUNIT);
 }
 
-// NOTE: We allow racy accesses to the instruction pointer from other threads
-// for sys._current_frames() and similar APIs.
 int _Py_NO_SANITIZE_THREAD
 PyUnstable_InterpreterFrame_GetLine(_PyInterpreterFrame *frame)
 {