Skip to content

Commit 44c1536

Browse files
gh-144446: Fix thread-safety of FrameLocalsProxy on executing frames
Synchronize FrameLocalsProxy accesses with the frame's owning thread: accessors run under the frame object's critical section and stop the world when the frame is executing on another thread. Co-authored-by: Sam Gross <colesbury@gmail.com>
1 parent e56f86f commit 44c1536

3 files changed

Lines changed: 533 additions & 51 deletions

File tree

Lib/test/test_free_threading/test_frame.py

Lines changed: 339 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import threading
44
import unittest
55

6-
from test.support import threading_helper
6+
from test.support import import_helper, threading_helper
77

88
threading_helper.requires_working_threading(module=True)
99

@@ -122,6 +122,344 @@ def writer(frame):
122122

123123
run_with_frame([reader, writer, reader, writer])
124124

125+
def test_concurrent_f_locals_read_values(self):
126+
def runner():
127+
a = 1
128+
b = "hello"
129+
c = [1, 2, 3]
130+
for i in range(100):
131+
a += i
132+
133+
def reader(frame):
134+
locals_dict = frame.f_locals
135+
list(locals_dict.keys())
136+
list(locals_dict.values())
137+
138+
run_with_frame(reader, runner=runner)
139+
140+
def test_concurrent_f_locals_write(self):
141+
def runner():
142+
x = 0
143+
for i in range(100):
144+
x += i
145+
146+
def writer(frame):
147+
frame.f_locals["new_var"] = 42
148+
149+
run_with_frame(writer, runner=runner)
150+
151+
def test_concurrent_f_locals_read_write(self):
152+
def runner():
153+
a = 1
154+
b = 2
155+
for i in range(100):
156+
a += i
157+
158+
def reader(frame):
159+
_ = frame.f_locals.get("a")
160+
_ = frame.f_locals.get("b")
161+
162+
def writer(frame):
163+
frame.f_locals["a"] = 42
164+
165+
run_with_frame([reader, writer, reader, writer], runner=runner)
166+
167+
def test_concurrent_f_locals_iteration(self):
168+
def runner():
169+
a = 1
170+
b = "hello"
171+
c = [1, 2, 3]
172+
for i in range(100):
173+
a += i
174+
175+
def iterator(frame):
176+
for key, value in frame.f_locals.items():
177+
pass
178+
179+
run_with_frame(iterator, runner=runner)
180+
181+
def test_gen_f_locals_read_while_running(self):
182+
# gh-144446: reading f_locals of a generator frame while the
183+
# generator is executing on another thread.
184+
for _ in range(5):
185+
def gen_fn():
186+
x = 0
187+
obj = None
188+
s = None
189+
yield
190+
for i in range(2000):
191+
obj = [i] * 4
192+
s = str(i) * 8
193+
x += i
194+
yield x
195+
196+
g = gen_fn()
197+
next(g)
198+
frame = g.gi_frame
199+
barrier = threading.Barrier(3)
200+
201+
def runner():
202+
barrier.wait()
203+
next(g)
204+
205+
def reader():
206+
barrier.wait()
207+
for _ in range(100):
208+
fl = frame.f_locals
209+
list(fl.values())
210+
fl.get("obj")
211+
fl.get("s")
212+
len(fl)
213+
214+
threading_helper.run_concurrently([runner, reader, reader])
215+
g.close()
216+
217+
def test_gen_f_locals_vs_resume_cycle(self):
218+
# Concurrent f_locals access must not make a concurrent send()
219+
# spuriously fail with "already executing".
220+
for _ in range(5):
221+
def gen_fn():
222+
x = 0
223+
while True:
224+
x += 1
225+
yield x
226+
227+
g = gen_fn()
228+
next(g)
229+
frame = g.gi_frame
230+
barrier = threading.Barrier(3)
231+
232+
def runner():
233+
barrier.wait()
234+
for _ in range(1000):
235+
next(g)
236+
237+
def reader():
238+
barrier.wait()
239+
for _ in range(200):
240+
fl = frame.f_locals
241+
fl.get("x")
242+
list(fl.items())
243+
244+
threading_helper.run_concurrently([runner, reader, reader])
245+
g.close()
246+
247+
def test_gen_f_locals_write_suspended(self):
248+
# Writes through f_locals must be synchronized with resuming.
249+
for _ in range(5):
250+
def gen_fn():
251+
x = 0
252+
extra = None
253+
while True:
254+
x += 1
255+
yield x
256+
257+
g = gen_fn()
258+
next(g)
259+
frame = g.gi_frame
260+
barrier = threading.Barrier(3)
261+
262+
def runner():
263+
barrier.wait()
264+
for _ in range(500):
265+
next(g)
266+
267+
def writer():
268+
barrier.wait()
269+
for i in range(200):
270+
frame.f_locals["extra"] = [i]
271+
frame.f_locals["new_var"] = i
272+
273+
threading_helper.run_concurrently([runner, writer, writer])
274+
g.close()
275+
276+
def test_gen_f_locals_inside_running_gen(self):
277+
# f_locals access from inside a running generator happens on the
278+
# executing thread itself and must work without synchronization
279+
# with other threads accessing the same frame.
280+
for _ in range(5):
281+
def gen_fn():
282+
x = 0
283+
yield
284+
frame = sys._getframe()
285+
for i in range(500):
286+
x += i
287+
assert frame.f_locals["x"] == x
288+
yield x
289+
290+
g = gen_fn()
291+
next(g)
292+
frame = g.gi_frame
293+
barrier = threading.Barrier(3)
294+
295+
def runner():
296+
barrier.wait()
297+
next(g)
298+
299+
def reader():
300+
barrier.wait()
301+
for _ in range(100):
302+
frame.f_locals.get("x")
303+
304+
threading_helper.run_concurrently([runner, reader, reader])
305+
g.close()
306+
307+
def test_gen_f_locals_dying_generator(self):
308+
# Access f_locals while the last reference to the generator is
309+
# dropped and the frame ownership moves to the frame object.
310+
for _ in range(20):
311+
def gen_fn():
312+
x = 42
313+
yield x
314+
315+
g = gen_fn()
316+
next(g)
317+
frame = g.gi_frame
318+
barrier = threading.Barrier(3)
319+
ref = [g]
320+
del g
321+
322+
def dropper():
323+
barrier.wait()
324+
ref.clear()
325+
326+
def reader():
327+
barrier.wait()
328+
for _ in range(100):
329+
frame.f_locals.get("x")
330+
list(frame.f_locals.values())
331+
332+
threading_helper.run_concurrently([dropper, reader, reader])
333+
334+
def test_setitem_old_value_destructor_reenters_proxy(self):
335+
# gh-144446: the value displaced by a f_locals store must be
336+
# released outside the synchronized region: its destructor may
337+
# access the proxy again (this would deadlock on the frame's
338+
# critical section, or try to stop the world twice).
339+
deleted = []
340+
frame = sys._getframe()
341+
342+
class Old:
343+
def __del__(self):
344+
deleted.append(frame.f_locals.get("marker"))
345+
346+
marker = 42
347+
# Not a real local: goes to the frame's extra locals dict.
348+
frame.f_locals["extra_key"] = Old()
349+
frame.f_locals["extra_key"] = None # replace: destructor runs
350+
self.assertEqual(deleted, [42])
351+
del frame.f_locals["extra_key"]
352+
353+
def test_gen_setitem_old_value_destructor_stw(self):
354+
# Same as above, but on a suspended generator frame, where the
355+
# store happens under stop-the-world.
356+
deleted = []
357+
358+
def gen_fn():
359+
yield
360+
361+
g = gen_fn()
362+
next(g)
363+
frame = g.gi_frame
364+
365+
class Old:
366+
def __del__(self):
367+
# Accessing the suspended generator frame's proxy stops
368+
# the world again; it must run after the world restarts.
369+
deleted.append(len(frame.f_locals))
370+
371+
frame.f_locals["extra_key"] = Old()
372+
frame.f_locals["extra_key"] = None
373+
self.assertEqual(len(deleted), 1)
374+
del frame.f_locals["extra_key"]
375+
g.close()
376+
377+
def test_gen_setitem_cell_old_value_destructor_stw(self):
378+
# The old value displaced from a cell variable must also be
379+
# released after the world restarts.
380+
deleted = []
381+
382+
def make_gen():
383+
x = None
384+
def gen_fn():
385+
nonlocal x
386+
yield x
387+
return gen_fn()
388+
389+
g = make_gen()
390+
next(g)
391+
frame = g.gi_frame
392+
393+
class Old:
394+
def __del__(self):
395+
deleted.append(frame.f_locals.get("x"))
396+
397+
frame.f_locals["x"] = Old()
398+
frame.f_locals["x"] = "new" # replace cell value: destructor runs
399+
self.assertEqual(deleted, ["new"])
400+
g.close()
401+
402+
def test_gen_pop_extra_locals_concurrent(self):
403+
# pop() must be synchronized with the frame's owner like the
404+
# other accessors.
405+
for _ in range(5):
406+
def gen_fn():
407+
x = 0
408+
while True:
409+
x += 1
410+
yield x
411+
412+
g = gen_fn()
413+
next(g)
414+
frame = g.gi_frame
415+
barrier = threading.Barrier(3)
416+
417+
def runner():
418+
barrier.wait()
419+
for _ in range(500):
420+
next(g)
421+
422+
def writer():
423+
barrier.wait()
424+
for i in range(200):
425+
frame.f_locals["extra_key"] = [i]
426+
frame.f_locals.pop("extra_key", None)
427+
428+
threading_helper.run_concurrently([runner, writer, writer])
429+
g.close()
430+
431+
def test_gen_getvar_while_running(self):
432+
# PyFrame_GetVar() reads fast locals and must synchronize with
433+
# the frame's owner as well.
434+
_testcapi = import_helper.import_module("_testcapi")
435+
for _ in range(5):
436+
def gen_fn():
437+
obj = None
438+
yield
439+
for i in range(2000):
440+
obj = [i] * 4
441+
yield obj
442+
443+
g = gen_fn()
444+
next(g)
445+
frame = g.gi_frame
446+
barrier = threading.Barrier(3)
447+
448+
def runner():
449+
barrier.wait()
450+
next(g)
451+
452+
def reader():
453+
barrier.wait()
454+
for _ in range(100):
455+
try:
456+
_testcapi.frame_getvar(frame, "obj")
457+
except NameError:
458+
pass
459+
460+
threading_helper.run_concurrently([runner, reader, reader])
461+
g.close()
462+
125463
def test_concurrent_frame_clear(self):
126464
# Test race between frame.clear() and attribute reads.
127465
def create_frame():
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix thread safety of :attr:`frame.f_locals` and :c:func:`PyFrame_GetVar` on
2+
frames executing on another thread in the free-threaded build.

0 commit comments

Comments
 (0)