Skip to content

Commit 367b372

Browse files
committed
gh-151640: Track sharing of BytesIO buffer in free-threaded builds
Keep zero-copy whole-buffer reads, but detach to a private buffer before a later write can mutate or resize an exposed BytesIO buffer.
1 parent 8b1dbb1 commit 367b372

4 files changed

Lines changed: 83 additions & 3 deletions

File tree

Lib/test/test_free_threading/test_io.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,35 @@ def sizeof(barrier, b, *ignore):
122122
class CBytesIOTest(ThreadSafetyMixin, TestCase):
123123
ioclass = io.BytesIO
124124

125+
@threading_helper.requires_working_threading()
126+
@threading_helper.reap_threads
127+
def test_concurrent_whole_buffer_read_and_resize(self):
128+
shared = self.ioclass(b"x" * 64)
129+
writers = 2
130+
readers = 8
131+
loops = 2000
132+
barrier = threading.Barrier(writers + readers)
133+
134+
def writer():
135+
barrier.wait()
136+
for i in range(loops):
137+
shared.seek(0)
138+
shared.write(b"a" * (64 + (i & 63)))
139+
140+
def reader():
141+
barrier.wait()
142+
for _ in range(loops):
143+
shared.seek(0)
144+
shared.read()
145+
shared.seek(0)
146+
shared.peek()
147+
shared.getvalue()
148+
149+
threads = [threading.Thread(target=writer) for _ in range(writers)]
150+
threads += [threading.Thread(target=reader) for _ in range(readers)]
151+
with threading_helper.start_threads(threads):
152+
pass
153+
125154
class PyBytesIOTest(ThreadSafetyMixin, TestCase):
126155
ioclass = pyio.BytesIO
127156

Lib/test/test_io/test_memoryio.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -939,7 +939,10 @@ def test_setstate(self):
939939

940940
@support.cpython_only
941941
def test_sizeof(self):
942-
basesize = support.calcobjsize('P2n2Pn')
942+
if support.Py_GIL_DISABLED:
943+
basesize = support.calcobjsize('P2n2Pni')
944+
else:
945+
basesize = support.calcobjsize('P2n2Pn')
943946
check = self.check_sizeof
944947
self.assertEqual(object.__sizeof__(io.BytesIO()), basesize)
945948
check(io.BytesIO(), basesize )
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix a data race in :class:`io.BytesIO` in free-threaded builds when
2+
whole-buffer reads or peeks, or :meth:`~io.BytesIO.getvalue`, share the
3+
internal buffer with concurrent writes.

Modules/_io/bytesio.c

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ typedef struct {
2222
PyObject *dict;
2323
PyObject *weakreflist;
2424
Py_ssize_t exports;
25+
#ifdef Py_GIL_DISABLED
26+
int buf_shared;
27+
#endif
2528
} bytesio;
2629

2730
#define bytesio_CAST(op) ((bytesio *)(op))
@@ -71,7 +74,45 @@ check_exports(bytesio *self)
7174
return NULL; \
7275
}
7376

77+
#ifdef Py_GIL_DISABLED
78+
#define SHARED_BUF(self) ((self)->buf_shared || !_PyObject_IsUniquelyReferenced((self)->buf))
79+
#else
7480
#define SHARED_BUF(self) (!_PyObject_IsUniquelyReferenced((self)->buf))
81+
#endif
82+
83+
static inline void
84+
set_shared_buf(bytesio *self)
85+
{
86+
#ifdef Py_GIL_DISABLED
87+
self->buf_shared = 1;
88+
#endif
89+
}
90+
91+
static inline void
92+
clear_shared_buf(bytesio *self)
93+
{
94+
#ifdef Py_GIL_DISABLED
95+
self->buf_shared = 0;
96+
#endif
97+
}
98+
99+
static int
100+
resize_unshared_buffer_lock_held(bytesio *self, Py_ssize_t size)
101+
{
102+
_Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(self);
103+
104+
#ifdef Py_GIL_DISABLED
105+
/* If the internal bytes object escaped via a zero-copy getvalue(), read(),
106+
or peek(), resizing it would mutate an object visible to Python code.
107+
Callers must detach first. */
108+
assert(!self->buf_shared);
109+
#endif
110+
int ret = _PyBytes_Resize(&self->buf, size);
111+
if (ret == 0) {
112+
clear_shared_buf(self);
113+
}
114+
return ret;
115+
}
75116

76117

77118
/* Internal routine to get a line from the buffer of a BytesIO
@@ -128,6 +169,7 @@ unshare_buffer_lock_held(bytesio *self, size_t size)
128169
memcpy(PyBytes_AS_STRING(new_buf), PyBytes_AS_STRING(self->buf),
129170
self->string_size);
130171
Py_SETREF(self->buf, new_buf);
172+
clear_shared_buf(self);
131173
return 0;
132174
}
133175

@@ -173,7 +215,7 @@ resize_buffer_lock_held(bytesio *self, size_t size)
173215
return -1;
174216
}
175217
else {
176-
if (_PyBytes_Resize(&self->buf, alloc) < 0)
218+
if (resize_unshared_buffer_lock_held(self, alloc) < 0)
177219
return -1;
178220
}
179221

@@ -381,10 +423,11 @@ _io_BytesIO_getvalue_impl(bytesio *self)
381423
return NULL;
382424
}
383425
else {
384-
if (_PyBytes_Resize(&self->buf, self->string_size) < 0)
426+
if (resize_unshared_buffer_lock_held(self, self->string_size) < 0)
385427
return NULL;
386428
}
387429
}
430+
set_shared_buf(self);
388431
return Py_NewRef(self->buf);
389432
}
390433

@@ -433,6 +476,7 @@ peek_bytes_lock_held(bytesio *self, Py_ssize_t size)
433476
if (size > 1 &&
434477
self->pos == 0 && size == PyBytes_GET_SIZE(self->buf) &&
435478
FT_ATOMIC_LOAD_SSIZE_RELAXED(self->exports) == 0) {
479+
set_shared_buf(self);
436480
return Py_NewRef(self->buf);
437481
}
438482

@@ -1091,6 +1135,7 @@ _io_BytesIO___init___impl(bytesio *self, PyObject *initvalue)
10911135
if (initvalue && initvalue != Py_None) {
10921136
if (PyBytes_CheckExact(initvalue)) {
10931137
Py_XSETREF(self->buf, Py_NewRef(initvalue));
1138+
clear_shared_buf(self);
10941139
self->string_size = PyBytes_GET_SIZE(initvalue);
10951140
}
10961141
else {

0 commit comments

Comments
 (0)