Skip to content

Commit 4269e2d

Browse files
weixluayaangazali
authored andcommitted
gh-127716: make memoryview thread-safe
Co-authored-by: ayaangazali <ayaangazali.work@gmail.com> Co-authored-by: Lu Xiaowei <weixlu420302@gmail.com>
1 parent 9721f8f commit 4269e2d

6 files changed

Lines changed: 780 additions & 71 deletions

File tree

Doc/library/threadsafety.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -604,3 +604,9 @@ underlying object supports it:
604604
Resizing or reallocating the underlying object (such as calling
605605
:meth:`bytearray.resize`) while a memoryview is exported raises
606606
:exc:`BufferError`. This is enforced regardless of threading.
607+
608+
In the :term:`free-threaded build`, :meth:`~memoryview.release` is
609+
thread-safe. If another thread has an in-flight operation on the memoryview
610+
when it is released, the underlying buffer may remain exported until that
611+
operation completes. Restrictions on the exporter, such as prohibiting
612+
resizing, remain in effect until then.

Include/cpython/memoryobject.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
typedef struct {
1212
PyObject_HEAD
1313
int flags; /* state flags */
14-
Py_ssize_t exports; /* number of direct memoryview exports */
14+
Py_ssize_t exports; /* registered views and active operations */
1515
Py_buffer master; /* snapshot buffer obtained from the original exporter */
1616
} _PyManagedBufferObject;
1717

Lines changed: 313 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,313 @@
1+
import pickle
2+
import threading
3+
import unittest
4+
5+
from test.support import threading_helper
6+
7+
NTHREADS = 8
8+
9+
# Fresh objects expose one-time races under ThreadSanitizer.
10+
ROUNDS = 20
11+
ITERS = 20
12+
13+
HASH_DATA_TEMPLATE = bytes(range(256))
14+
HASH_DATA_REPEAT = 256
15+
16+
READ_OPS = [
17+
lambda mv: mv.tobytes(),
18+
lambda mv: mv.hex(),
19+
lambda mv: mv.tolist(),
20+
lambda mv: mv.cast("B"),
21+
lambda mv: mv.toreadonly(),
22+
lambda mv: memoryview(mv),
23+
lambda mv: bytes(mv),
24+
lambda mv: pickle.PickleBuffer(mv).release(),
25+
lambda mv: mv.obj,
26+
lambda mv: mv.format,
27+
lambda mv: mv.c_contiguous,
28+
lambda mv: mv[0],
29+
lambda mv: mv[0:4],
30+
lambda mv: mv.count(0),
31+
lambda mv: mv.index(0),
32+
lambda mv: mv == mv,
33+
lambda mv: list(mv),
34+
lambda mv: len(mv),
35+
]
36+
37+
38+
def run_racy(func, *args):
39+
try:
40+
func(*args)
41+
except (ValueError, BufferError):
42+
pass
43+
44+
45+
@threading_helper.requires_working_threading()
46+
class TestMemoryViewRaces(unittest.TestCase):
47+
def assert_exporter_free(self, buf):
48+
buf.append(0)
49+
del buf[-1]
50+
51+
def test_concurrent_slicing_keeps_export_count(self):
52+
for _ in range(ROUNDS):
53+
mv = memoryview(bytes(64))
54+
slices = []
55+
lock = threading.Lock()
56+
57+
def make_slices():
58+
local = [mv[0:4] for _ in range(ITERS)]
59+
local += [memoryview(mv) for _ in range(ITERS)]
60+
with lock:
61+
slices.extend(local)
62+
63+
threading_helper.run_concurrently(make_slices, nthreads=NTHREADS)
64+
del slices
65+
66+
self.assertEqual(bytes(mv[0:4]), b"\x00" * 4)
67+
68+
def test_concurrent_release(self):
69+
buf = bytearray(64)
70+
71+
for _ in range(ROUNDS):
72+
views = [memoryview(buf) for _ in range(NTHREADS)]
73+
74+
def release(views=views):
75+
for mv in views:
76+
run_racy(mv.release)
77+
78+
threading_helper.run_concurrently(release, nthreads=NTHREADS)
79+
80+
self.assert_exporter_free(buf)
81+
82+
def test_release_races_with_reads(self):
83+
for _ in range(ROUNDS):
84+
buf = bytearray(64)
85+
cell = [memoryview(buf)]
86+
lock = threading.Lock()
87+
88+
def releaser():
89+
for _ in range(ITERS):
90+
mv = cell[0]
91+
run_racy(mv.release)
92+
with lock:
93+
cell[0] = memoryview(buf)
94+
95+
def reader():
96+
for _ in range(ITERS):
97+
mv = cell[0]
98+
for op in READ_OPS:
99+
run_racy(op, mv)
100+
101+
threading_helper.run_concurrently(
102+
[releaser] * (NTHREADS // 2) + [reader] * (NTHREADS // 2),
103+
nthreads=NTHREADS,
104+
)
105+
106+
cell[0].release()
107+
self.assert_exporter_free(buf)
108+
109+
def test_read_keeps_exporter_alive_after_release(self):
110+
size = 1 << 20
111+
for _ in range(ROUNDS):
112+
exporter = [bytearray(size)]
113+
view = memoryview(exporter[0])
114+
stale = []
115+
116+
def reader():
117+
try:
118+
for _ in range(4):
119+
data = view.tobytes()
120+
if data.count(0) != len(data):
121+
stale.append(True)
122+
return
123+
except ValueError:
124+
pass
125+
126+
def releaser():
127+
view.release()
128+
exporter.clear()
129+
for _ in range(8):
130+
bytearray(b"\xdb" * size)
131+
132+
threading_helper.run_concurrently([reader, releaser])
133+
self.assertFalse(stale)
134+
135+
def test_release_races_with_writes(self):
136+
for _ in range(ROUNDS):
137+
buf = bytearray(8 * NTHREADS)
138+
cell = [memoryview(buf)]
139+
lock = threading.Lock()
140+
141+
def releaser():
142+
for _ in range(ITERS):
143+
mv = cell[0]
144+
run_racy(mv.release)
145+
with lock:
146+
cell[0] = memoryview(buf)
147+
148+
def writer(slot):
149+
start = slot * 8
150+
for _ in range(ITERS):
151+
mv = cell[0]
152+
run_racy(mv.__setitem__, start, 1)
153+
run_racy(mv.__setitem__, slice(start, start + 4), b"abcd")
154+
155+
workers = [releaser] * (NTHREADS // 2)
156+
workers += [lambda s=s: writer(s) for s in range(NTHREADS // 2)]
157+
threading_helper.run_concurrently(workers, nthreads=NTHREADS)
158+
159+
cell[0].release()
160+
self.assert_exporter_free(buf)
161+
162+
def test_release_races_with_buffer_exports(self):
163+
for _ in range(ROUNDS):
164+
buf = bytearray(64)
165+
mv = memoryview(buf)
166+
167+
def exporter():
168+
for _ in range(ITERS):
169+
run_racy(lambda: pickle.PickleBuffer(mv).release())
170+
171+
def releaser():
172+
for _ in range(ITERS):
173+
run_racy(mv.release)
174+
175+
threading_helper.run_concurrently(
176+
[exporter] * (NTHREADS - 1) + [releaser], nthreads=NTHREADS
177+
)
178+
179+
mv.release()
180+
self.assert_exporter_free(buf)
181+
182+
def test_release_with_live_export(self):
183+
buf = bytearray(64)
184+
185+
for _ in range(ROUNDS):
186+
mv = memoryview(buf)
187+
held = pickle.PickleBuffer(mv)
188+
189+
def release(mv=mv):
190+
try:
191+
mv.release()
192+
except BufferError:
193+
pass
194+
195+
threading_helper.run_concurrently(release, nthreads=NTHREADS)
196+
197+
self.assertEqual(bytes(mv[0:4]), b"\x00" * 4)
198+
held.release()
199+
mv.release()
200+
201+
self.assert_exporter_free(buf)
202+
203+
def test_compare_two_views_races_with_release(self):
204+
for _ in range(ROUNDS):
205+
buf = bytearray(64)
206+
cell = [memoryview(buf), memoryview(buf)]
207+
lock = threading.Lock()
208+
209+
def releaser(slot):
210+
for _ in range(ITERS):
211+
mv = cell[slot]
212+
run_racy(mv.release)
213+
with lock:
214+
cell[slot] = memoryview(buf)
215+
216+
def comparer():
217+
for _ in range(ITERS):
218+
left, right = cell[0], cell[1]
219+
run_racy(lambda: left == right)
220+
run_racy(lambda: left != right)
221+
222+
threading_helper.run_concurrently(
223+
[lambda: releaser(0), lambda: releaser(1)] + [comparer] * 6,
224+
nthreads=NTHREADS,
225+
)
226+
227+
for mv in cell:
228+
mv.release()
229+
self.assert_exporter_free(buf)
230+
231+
def test_release_parent_keeps_child_valid(self):
232+
for _ in range(ROUNDS):
233+
buf = bytearray(range(64))
234+
parent = memoryview(buf)
235+
child = parent[0:32]
236+
237+
def use_child():
238+
for _ in range(ITERS):
239+
self.assertEqual(child[0], 0)
240+
child.tobytes()
241+
242+
def release_parent():
243+
parent.release()
244+
245+
threading_helper.run_concurrently(
246+
[release_parent] + [use_child] * (NTHREADS - 1),
247+
nthreads=NTHREADS,
248+
)
249+
250+
self.assertEqual(child.tobytes(), bytes(range(32)))
251+
child.release()
252+
self.assert_exporter_free(buf)
253+
254+
def test_concurrent_iteration(self):
255+
for _ in range(ROUNDS):
256+
buf = bytearray(range(64))
257+
cell = [memoryview(buf)]
258+
lock = threading.Lock()
259+
260+
def releaser():
261+
for _ in range(ITERS):
262+
mv = cell[0]
263+
run_racy(mv.release)
264+
with lock:
265+
cell[0] = memoryview(buf)
266+
267+
def iterator():
268+
for _ in range(ITERS):
269+
run_racy(list, cell[0])
270+
271+
threading_helper.run_concurrently(
272+
[releaser] * (NTHREADS // 2) + [iterator] * (NTHREADS // 2),
273+
nthreads=NTHREADS,
274+
)
275+
276+
cell[0].release()
277+
self.assert_exporter_free(buf)
278+
279+
def test_iterator_exhaustion_drops_last_reference(self):
280+
def loop():
281+
for _ in range(ROUNDS * ITERS):
282+
self.assertEqual(list(iter(memoryview(b"ab"))), [97, 98])
283+
284+
threading_helper.run_concurrently(loop, nthreads=NTHREADS)
285+
286+
def test_concurrent_hash(self):
287+
for _ in range(ROUNDS):
288+
data = HASH_DATA_TEMPLATE * HASH_DATA_REPEAT
289+
mv = memoryview(data)
290+
results = []
291+
lock = threading.Lock()
292+
293+
def hasher():
294+
local = {hash(mv) for _ in range(ITERS)}
295+
with lock:
296+
results.append(local)
297+
298+
threading_helper.run_concurrently(hasher, nthreads=NTHREADS)
299+
self.assertEqual({h for s in results for h in s}, {hash(data)})
300+
301+
def test_concurrent_hash_and_release(self):
302+
for _ in range(ROUNDS):
303+
mv = memoryview(HASH_DATA_TEMPLATE * HASH_DATA_REPEAT)
304+
305+
def work(mv=mv):
306+
run_racy(hash, mv)
307+
run_racy(mv.release)
308+
309+
threading_helper.run_concurrently(work, nthreads=NTHREADS)
310+
311+
312+
if __name__ == "__main__":
313+
unittest.main()

Lib/test/test_memoryview.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,13 @@ def test_iter(self):
9191
m = self._view(b)
9292
self.assertEqual(list(m), [m[i] for i in range(len(m))])
9393

94+
def test_released_empty_iterator(self):
95+
for tp in self._types:
96+
m = self._view(tp(b""))
97+
iterator = iter(m)
98+
m.release()
99+
self.assertRaises(StopIteration, next, iterator)
100+
94101
def test_count(self):
95102
for tp in self._types:
96103
b = tp(self._source)
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
On the free-threaded build, :class:`memoryview` operations are now thread safe:
2+
an operation that reads the buffer holds the managed buffer for its duration,
3+
so a concurrent :meth:`~memoryview.release` can no longer free the buffer under
4+
it; slicing a memoryview from several threads no longer loses buffer
5+
registrations and raises ``ValueError: operation forbidden on released
6+
memoryview object``; and :func:`hash` of a memoryview is no longer a data race.
7+
As a result :meth:`~memoryview.release` may now return while an operation
8+
started by another thread is still running, see
9+
:ref:`thread-safety-memoryview`.

0 commit comments

Comments
 (0)