Skip to content

Commit 2f5e017

Browse files
committed
gh-153852: Synchronize frame tracing state in the free-threaded build
1 parent 1a85213 commit 2f5e017

7 files changed

Lines changed: 214 additions & 12 deletions

File tree

‎Lib/test/test_free_threading/test_frame.py‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import threading
44
import unittest
55

6+
from test import support
67
from test.support import threading_helper
78

89
threading_helper.requires_working_threading(module=True)
@@ -104,6 +105,84 @@ def writer(frame):
104105

105106
run_with_frame([reader, writer, reader, writer])
106107

108+
def test_concurrent_f_trace_while_tracing(self):
109+
frame_var = None
110+
ready = threading.Event()
111+
start = threading.Barrier(2, timeout=support.SHORT_TIMEOUT)
112+
113+
def trace(frame, event, arg):
114+
if frame.f_code is runner.__code__:
115+
return trace
116+
117+
def runner():
118+
nonlocal frame_var
119+
frame_var = sys._getframe()
120+
ready.set()
121+
start.wait()
122+
for _ in range(100):
123+
pass
124+
125+
def executor():
126+
try:
127+
sys.settrace(trace)
128+
runner()
129+
finally:
130+
sys.settrace(None)
131+
132+
def reader():
133+
self.assertTrue(ready.wait(support.SHORT_TIMEOUT))
134+
frame = frame_var
135+
start.wait()
136+
for _ in range(100):
137+
self.assertIs(frame.f_trace, trace)
138+
139+
threading_helper.run_concurrently([executor, reader])
140+
141+
def _test_concurrent_trace_flag_while_tracing(self, name):
142+
frame_var = None
143+
ready = threading.Event()
144+
start = threading.Barrier(2, timeout=support.SHORT_TIMEOUT)
145+
146+
def trace(frame, event, arg):
147+
if frame.f_code is runner.__code__:
148+
return trace
149+
150+
def runner():
151+
nonlocal frame_var
152+
frame_var = sys._getframe()
153+
ready.set()
154+
start.wait()
155+
for _ in range(100):
156+
pass
157+
158+
def executor():
159+
try:
160+
sys.settrace(trace)
161+
runner()
162+
finally:
163+
sys.settrace(None)
164+
165+
def writer():
166+
self.assertTrue(ready.wait(support.SHORT_TIMEOUT))
167+
frame = frame_var
168+
start.wait()
169+
original = getattr(frame, name)
170+
try:
171+
for _ in range(100):
172+
for value in (True, False):
173+
setattr(frame, name, value)
174+
self.assertIs(getattr(frame, name), value)
175+
finally:
176+
setattr(frame, name, original)
177+
178+
threading_helper.run_concurrently([executor, writer])
179+
180+
def test_concurrent_f_trace_lines_while_tracing(self):
181+
self._test_concurrent_trace_flag_while_tracing('f_trace_lines')
182+
183+
def test_concurrent_f_trace_opcodes_while_tracing(self):
184+
self._test_concurrent_trace_flag_while_tracing('f_trace_opcodes')
185+
107186
def test_concurrent_f_trace_opcodes_write(self):
108187
def writer(frame):
109188
frame.f_trace_opcodes = True

‎Lib/test/test_sys_settrace.py‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3074,6 +3074,114 @@ def foo(*args):
30743074
del foo
30753075
sys.settrace(sys.gettrace())
30763076

3077+
def test_local_trace_replacement(self):
3078+
events = []
3079+
3080+
def assigned(frame, event, arg):
3081+
events.append(('assigned', event))
3082+
return assigned
3083+
3084+
def replacement(frame, event, arg):
3085+
events.append(('replacement', event))
3086+
return replacement
3087+
3088+
def local_trace(frame, event, arg):
3089+
events.append(('original', event))
3090+
frame.f_trace = assigned
3091+
return replacement
3092+
3093+
def trace(frame, event, arg):
3094+
if frame.f_code is target.__code__:
3095+
return local_trace
3096+
3097+
def target():
3098+
value = 1
3099+
return value
3100+
3101+
sys.settrace(trace)
3102+
target()
3103+
sys.settrace(None)
3104+
self.assertEqual(events, [('original', 'line'),
3105+
('replacement', 'line'),
3106+
('replacement', 'return')])
3107+
3108+
def test_local_trace_clear(self):
3109+
events = []
3110+
3111+
def local_trace(frame, event, arg):
3112+
events.append(event)
3113+
frame.f_trace = None
3114+
return None
3115+
3116+
def trace(frame, event, arg):
3117+
if frame.f_code is target.__code__:
3118+
return local_trace
3119+
3120+
def target():
3121+
value = 1
3122+
return value
3123+
3124+
sys.settrace(trace)
3125+
target()
3126+
sys.settrace(None)
3127+
self.assertEqual(events, ['line'])
3128+
3129+
def test_local_trace_error(self):
3130+
frames = []
3131+
3132+
def local_trace(frame, event, arg):
3133+
raise RuntimeError('local trace error')
3134+
3135+
def trace(frame, event, arg):
3136+
if frame.f_code is target.__code__:
3137+
frames.append(frame)
3138+
return local_trace
3139+
3140+
def target():
3141+
return 1
3142+
3143+
sys.settrace(trace)
3144+
with self.assertRaisesRegex(RuntimeError, 'local trace error'):
3145+
target()
3146+
self.assertIsNone(sys.gettrace())
3147+
self.assertEqual(len(frames), 1)
3148+
self.assertIsNone(frames[0].f_trace)
3149+
3150+
def test_local_trace_finalizer_reentrancy(self):
3151+
events = []
3152+
3153+
def final_trace(frame, event, arg):
3154+
events.append(('final', event))
3155+
return final_trace
3156+
3157+
def replacement(frame, event, arg):
3158+
events.append(('replacement', event))
3159+
return replacement
3160+
3161+
class LocalTrace:
3162+
def __call__(self, frame, event, arg):
3163+
self.frame = frame
3164+
events.append(('original', event))
3165+
return replacement
3166+
3167+
def __del__(self):
3168+
events.append(('finalize', self.frame.f_trace is replacement))
3169+
self.frame.f_trace = final_trace
3170+
3171+
def trace(frame, event, arg):
3172+
if frame.f_code is target.__code__:
3173+
return LocalTrace()
3174+
3175+
def target():
3176+
value = 1
3177+
return value
3178+
3179+
sys.settrace(trace)
3180+
target()
3181+
sys.settrace(None)
3182+
self.assertEqual(events, [('original', 'line'), ('finalize', True),
3183+
('final', 'line'), ('final', 'return')])
3184+
30773185

30783186
class TestLinesAfterTraceStarted(TraceTestCase):
30793187

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix data races in frame tracing state in the free-threaded build. Protect
2+
access to the frame trace callback and use atomic accesses for line and opcode
3+
tracing flags.

‎Objects/frameobject.c‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1212,7 +1212,7 @@ static PyObject *
12121212
frame_trace_opcodes_get_impl(PyFrameObject *self)
12131213
/*[clinic end generated code: output=53ff41d09cc32e87 input=4eb91dc88e04677a]*/
12141214
{
1215-
return self->f_trace_opcodes ? Py_True : Py_False;
1215+
return FT_ATOMIC_LOAD_CHAR_RELAXED(self->f_trace_opcodes) ? Py_True : Py_False;
12161216
}
12171217

12181218
/*[clinic input]
@@ -1231,13 +1231,13 @@ frame_trace_opcodes_set_impl(PyFrameObject *self, PyObject *value)
12311231
return -1;
12321232
}
12331233
if (value == Py_True) {
1234-
self->f_trace_opcodes = 1;
1234+
FT_ATOMIC_STORE_CHAR_RELAXED(self->f_trace_opcodes, 1);
12351235
if (self->f_trace) {
12361236
return _PyEval_SetOpcodeTrace(self, true);
12371237
}
12381238
}
12391239
else {
1240-
self->f_trace_opcodes = 0;
1240+
FT_ATOMIC_STORE_CHAR_RELAXED(self->f_trace_opcodes, 0);
12411241
return _PyEval_SetOpcodeTrace(self, false);
12421242
}
12431243
return 0;
@@ -1956,7 +1956,8 @@ frame_trace_set_impl(PyFrameObject *self, PyObject *value)
19561956
}
19571957
if (value != self->f_trace) {
19581958
Py_XSETREF(self->f_trace, Py_XNewRef(value));
1959-
if (value != NULL && self->f_trace_opcodes) {
1959+
if (value != NULL &&
1960+
FT_ATOMIC_LOAD_CHAR_RELAXED(self->f_trace_opcodes)) {
19601961
return _PyEval_SetOpcodeTrace(self, true);
19611962
}
19621963
}

‎Python/instrumentation.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1367,7 +1367,7 @@ _Py_call_instrumentation_line(PyThreadState *tstate, _PyInterpreterFrame* frame,
13671367
if (frame_obj == NULL) {
13681368
return -1;
13691369
}
1370-
if (frame_obj->f_trace_lines) {
1370+
if (FT_ATOMIC_LOAD_CHAR_RELAXED(frame_obj->f_trace_lines)) {
13711371
/* Need to set tracing and what_event as if using
13721372
* the instrumentation call. */
13731373
int old_what = tstate->what_event;

‎Python/legacy_tracing.c‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include "pycore_ceval.h" // export _PyEval_SetProfile()
88
#include "pycore_frame.h" // PyFrameObject members
99
#include "pycore_interpframe.h" // _PyFrame_GetCode()
10+
#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_CHAR_RELAXED()
1011

1112
#include "opcode.h"
1213
#include <stddef.h>
@@ -187,7 +188,7 @@ call_trace_func(_PyLegacyEventHandler *self, PyObject *arg)
187188
"Missing frame when calling trace function.");
188189
return NULL;
189190
}
190-
if (frame->f_trace_opcodes) {
191+
if (FT_ATOMIC_LOAD_CHAR_RELAXED(frame->f_trace_opcodes)) {
191192
if (_PyEval_SetOpcodeTrace(frame, true) != 0) {
192193
return NULL;
193194
}
@@ -302,7 +303,8 @@ sys_trace_instruction_func(
302303
return NULL;
303304
}
304305
PyThreadState *tstate = _PyThreadState_GET();
305-
if (!tstate->c_tracefunc || !frame->f_trace_opcodes) {
306+
if (!tstate->c_tracefunc ||
307+
!FT_ATOMIC_LOAD_CHAR_RELAXED(frame->f_trace_opcodes)) {
306308
if (_PyEval_SetOpcodeTrace(frame, false) != 0) {
307309
return NULL;
308310
}
@@ -323,7 +325,7 @@ trace_line(
323325
PyThreadState *tstate, _PyLegacyEventHandler *self,
324326
PyFrameObject *frame, int line
325327
) {
326-
if (!frame->f_trace_lines) {
328+
if (!FT_ATOMIC_LOAD_CHAR_RELAXED(frame->f_trace_lines)) {
327329
Py_RETURN_NONE;
328330
}
329331
if (line < 0) {
@@ -403,7 +405,7 @@ sys_trace_jump_func(
403405
"Missing frame when calling trace function.");
404406
return NULL;
405407
}
406-
if (!frame->f_trace_lines) {
408+
if (!FT_ATOMIC_LOAD_CHAR_RELAXED(frame->f_trace_lines)) {
407409
Py_RETURN_NONE;
408410
}
409411
return trace_line(tstate, self, frame, to_line);
@@ -680,7 +682,8 @@ maybe_set_opcode_trace(PyThreadState *tstate)
680682
return 0;
681683
}
682684
PyFrameObject *frame = iframe->frame_obj;
683-
if (frame == NULL || !frame->f_trace_opcodes) {
685+
if (frame == NULL ||
686+
!FT_ATOMIC_LOAD_CHAR_RELAXED(frame->f_trace_opcodes)) {
684687
return 0;
685688
}
686689
return set_opcode_trace_world_stopped(_PyFrame_GetCode(iframe), true);

‎Python/sysmodule.c‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1104,25 +1104,33 @@ trace_trampoline(PyObject *self, PyFrameObject *frame,
11041104
{
11051105
PyObject *callback;
11061106
if (what == PyTrace_CALL) {
1107-
callback = self;
1107+
callback = Py_XNewRef(self);
11081108
}
11091109
else {
1110-
callback = frame->f_trace;
1110+
Py_BEGIN_CRITICAL_SECTION(frame);
1111+
callback = Py_XNewRef(frame->f_trace);
1112+
Py_END_CRITICAL_SECTION();
11111113
}
11121114
if (callback == NULL) {
11131115
return 0;
11141116
}
11151117

11161118
PyThreadState *tstate = _PyThreadState_GET();
1119+
/* The callback can change f_trace or release the thread state. */
11171120
PyObject *result = call_trampoline(tstate, callback, frame, what, arg);
1121+
Py_DECREF(callback);
11181122
if (result == NULL) {
11191123
_PyEval_SetTrace(tstate, NULL, NULL);
1124+
Py_BEGIN_CRITICAL_SECTION(frame);
11201125
Py_CLEAR(frame->f_trace);
1126+
Py_END_CRITICAL_SECTION();
11211127
return -1;
11221128
}
11231129

11241130
if (result != Py_None) {
1131+
Py_BEGIN_CRITICAL_SECTION(frame);
11251132
Py_XSETREF(frame->f_trace, result);
1133+
Py_END_CRITICAL_SECTION();
11261134
}
11271135
else {
11281136
Py_DECREF(result);

0 commit comments

Comments
 (0)