Skip to content

Commit 4937ba5

Browse files
authored
gh-127085: fix some data races in memoryview in free-threading (#127412)
1 parent 1d276ec commit 4937ba5

File tree

3 files changed

+52
-8
lines changed

3 files changed

+52
-8
lines changed

Lib/test/test_memoryview.py

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616
import struct
1717

1818
from itertools import product
19-
from test.support import import_helper
19+
from test import support
20+
from test.support import import_helper, threading_helper
2021

2122

2223
class MyObject:
@@ -733,5 +734,28 @@ def test_picklebuffer_reference_loop(self):
733734
self.assertIsNone(wr())
734735

735736

737+
@threading_helper.requires_working_threading()
738+
@support.requires_resource("cpu")
739+
class RacingTest(unittest.TestCase):
740+
def test_racing_getbuf_and_releasebuf(self):
741+
"""Repeatly access the memoryview for racing."""
742+
from multiprocessing.managers import SharedMemoryManager
743+
from threading import Thread
744+
745+
n = 100
746+
with SharedMemoryManager() as smm:
747+
obj = smm.ShareableList(range(100))
748+
threads = []
749+
for _ in range(n):
750+
# Issue gh-127085, the `ShareableList.count` is just a convenient way to mess the `exports`
751+
# counter of `memoryview`, this issue has no direct relation with `ShareableList`.
752+
threads.append(Thread(target=obj.count, args=(1,)))
753+
754+
with threading_helper.start_threads(threads):
755+
pass
756+
757+
del obj
758+
759+
736760
if __name__ == "__main__":
737761
unittest.main()
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix race when exporting a buffer from a :class:`memoryview` object on the :term:`free-threaded <free threading>` build.

Objects/memoryobject.c

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1086,6 +1086,16 @@ PyBuffer_ToContiguous(void *buf, const Py_buffer *src, Py_ssize_t len, char orde
10861086
return ret;
10871087
}
10881088

1089+
static inline Py_ssize_t
1090+
get_exports(PyMemoryViewObject *buf)
1091+
{
1092+
#ifdef Py_GIL_DISABLED
1093+
return _Py_atomic_load_ssize_relaxed(&buf->exports);
1094+
#else
1095+
return buf->exports;
1096+
#endif
1097+
}
1098+
10891099

10901100
/****************************************************************************/
10911101
/* Release/GC management */
@@ -1098,7 +1108,7 @@ PyBuffer_ToContiguous(void *buf, const Py_buffer *src, Py_ssize_t len, char orde
10981108
static void
10991109
_memory_release(PyMemoryViewObject *self)
11001110
{
1101-
assert(self->exports == 0);
1111+
assert(get_exports(self) == 0);
11021112
if (self->flags & _Py_MEMORYVIEW_RELEASED)
11031113
return;
11041114

@@ -1119,15 +1129,16 @@ static PyObject *
11191129
memoryview_release_impl(PyMemoryViewObject *self)
11201130
/*[clinic end generated code: output=d0b7e3ba95b7fcb9 input=bc71d1d51f4a52f0]*/
11211131
{
1122-
if (self->exports == 0) {
1132+
Py_ssize_t exports = get_exports(self);
1133+
if (exports == 0) {
11231134
_memory_release(self);
11241135
Py_RETURN_NONE;
11251136
}
11261137

1127-
if (self->exports > 0) {
1138+
if (exports > 0) {
11281139
PyErr_Format(PyExc_BufferError,
1129-
"memoryview has %zd exported buffer%s", self->exports,
1130-
self->exports==1 ? "" : "s");
1140+
"memoryview has %zd exported buffer%s", exports,
1141+
exports==1 ? "" : "s");
11311142
return NULL;
11321143
}
11331144

@@ -1140,7 +1151,7 @@ static void
11401151
memory_dealloc(PyObject *_self)
11411152
{
11421153
PyMemoryViewObject *self = (PyMemoryViewObject *)_self;
1143-
assert(self->exports == 0);
1154+
assert(get_exports(self) == 0);
11441155
_PyObject_GC_UNTRACK(self);
11451156
_memory_release(self);
11461157
Py_CLEAR(self->mbuf);
@@ -1161,7 +1172,7 @@ static int
11611172
memory_clear(PyObject *_self)
11621173
{
11631174
PyMemoryViewObject *self = (PyMemoryViewObject *)_self;
1164-
if (self->exports == 0) {
1175+
if (get_exports(self) == 0) {
11651176
_memory_release(self);
11661177
Py_CLEAR(self->mbuf);
11671178
}
@@ -1589,7 +1600,11 @@ memory_getbuf(PyObject *_self, Py_buffer *view, int flags)
15891600

15901601

15911602
view->obj = Py_NewRef(self);
1603+
#ifdef Py_GIL_DISABLED
1604+
_Py_atomic_add_ssize(&self->exports, 1);
1605+
#else
15921606
self->exports++;
1607+
#endif
15931608

15941609
return 0;
15951610
}
@@ -1598,7 +1613,11 @@ static void
15981613
memory_releasebuf(PyObject *_self, Py_buffer *view)
15991614
{
16001615
PyMemoryViewObject *self = (PyMemoryViewObject *)_self;
1616+
#ifdef Py_GIL_DISABLED
1617+
_Py_atomic_add_ssize(&self->exports, -1);
1618+
#else
16011619
self->exports--;
1620+
#endif
16021621
return;
16031622
/* PyBuffer_Release() decrements view->obj after this function returns. */
16041623
}

0 commit comments

Comments
 (0)