[3.14] gh-144446: Fix some frame object thread-safety issues (gh-144479) (#144546)

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
(cherry picked from commit 5bb3bbb)

Co-authored-by: Sam Gross <colesbury@gmail.com>

Author: miss-islington

Lib/test/support/threading_helper.py

@@ -250,21 +250,32 @@ def requires_working_threading(*, module=False):
         return unittest.skipUnless(can_start_thread, msg)
 
 
-def run_concurrently(worker_func, nthreads, args=(), kwargs={}):
+def run_concurrently(worker_func, nthreads=None, args=(), kwargs={}):
     """
-    Run the worker function concurrently in multiple threads.
+    Run the worker function(s) concurrently in multiple threads.
+
+    If `worker_func` is a single callable, it is used for all threads.
+    If it is a list of callables, each callable is used for one thread.
     """
+    from collections.abc import Iterable
+
+    if nthreads is None:
+        nthreads = len(worker_func)
+    if not isinstance(worker_func, Iterable):
+        worker_func = [worker_func] * nthreads
+    assert len(worker_func) == nthreads
+
     barrier = threading.Barrier(nthreads)
 
-    def wrapper_func(*args, **kwargs):
+    def wrapper_func(func, *args, **kwargs):
         # Wait for all threads to reach this point before proceeding.
         barrier.wait()
-        worker_func(*args, **kwargs)
+        func(*args, **kwargs)
 
     with catch_threading_exception() as cm:
         workers = [
-            threading.Thread(target=wrapper_func, args=args, kwargs=kwargs)
-            for _ in range(nthreads)
+            threading.Thread(target=wrapper_func, args=(func, *args), kwargs=kwargs)
+            for func in worker_func
         ]
         with start_threads(workers):
             pass

Lib/test/test_free_threading/test_frame.py

@@ -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()

Misc/NEWS.d/next/Core_and_Builtins/2026-02-03-17-08-13.gh-issue-144446.db5619.rst

@@ -0,0 +1,2 @@
+Fix data races in the free-threaded build when reading frame object attributes
+while another thread is executing the frame.

Objects/frameobject.c

@@ -1036,11 +1036,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]
@@ -2044,11 +2044,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[] = {

Python/frame.c

@@ -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)
 {