Skip to content

gh-127085: Fix memoryview->exports race condition. #127412

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion Lib/test/test_memoryview.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
import struct

from itertools import product
from test.support import import_helper
from test import support
from test.support import import_helper, threading_helper


class MyObject:
Expand Down Expand Up @@ -733,5 +734,28 @@ def test_picklebuffer_reference_loop(self):
self.assertIsNone(wr())


@threading_helper.requires_working_threading()
@support.requires_resource("cpu")
class RacingTest(unittest.TestCase):
def test_racing_getbuf_and_releasebuf(self):
"""Repeatly access the memoryview for racing."""
from multiprocessing.managers import SharedMemoryManager
from threading import Thread

n = 100
with SharedMemoryManager() as smm:
obj = smm.ShareableList(range(100))
threads = []
for _ in range(n):
# Issue gh-127085, the `ShareableList.count` is just a convenient way to mess the `exports`
# counter of `memoryview`, this issue has no direct relation with `ShareableList`.
threads.append(Thread(target=obj.count, args=(1,)))

with threading_helper.start_threads(threads):
pass

del obj


if __name__ == "__main__":
unittest.main()
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix race when exporting a buffer from a :class:`memoryview` object on the :term:`free-threaded <free threading>` build.
33 changes: 26 additions & 7 deletions Objects/memoryobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -1086,6 +1086,16 @@ PyBuffer_ToContiguous(void *buf, const Py_buffer *src, Py_ssize_t len, char orde
return ret;
}

static inline Py_ssize_t
get_exports(PyMemoryViewObject *buf)
{
#ifdef Py_GIL_DISABLED
return _Py_atomic_load_ssize_relaxed(&buf->exports);
#else
return buf->exports;
#endif
}


/****************************************************************************/
/* Release/GC management */
Expand All @@ -1098,7 +1108,7 @@ PyBuffer_ToContiguous(void *buf, const Py_buffer *src, Py_ssize_t len, char orde
static void
_memory_release(PyMemoryViewObject *self)
{
assert(self->exports == 0);
assert(get_exports(self) == 0);
if (self->flags & _Py_MEMORYVIEW_RELEASED)
return;

Expand All @@ -1119,15 +1129,16 @@ static PyObject *
memoryview_release_impl(PyMemoryViewObject *self)
/*[clinic end generated code: output=d0b7e3ba95b7fcb9 input=bc71d1d51f4a52f0]*/
{
if (self->exports == 0) {
Py_ssize_t exports = get_exports(self);
if (exports == 0) {
_memory_release(self);
Py_RETURN_NONE;
}

if (self->exports > 0) {
if (exports > 0) {
PyErr_Format(PyExc_BufferError,
"memoryview has %zd exported buffer%s", self->exports,
self->exports==1 ? "" : "s");
"memoryview has %zd exported buffer%s", exports,
exports==1 ? "" : "s");
return NULL;
}

Expand All @@ -1140,7 +1151,7 @@ static void
memory_dealloc(PyObject *_self)
{
PyMemoryViewObject *self = (PyMemoryViewObject *)_self;
assert(self->exports == 0);
assert(get_exports(self) == 0);
_PyObject_GC_UNTRACK(self);
_memory_release(self);
Py_CLEAR(self->mbuf);
Expand All @@ -1161,7 +1172,7 @@ static int
memory_clear(PyObject *_self)
{
PyMemoryViewObject *self = (PyMemoryViewObject *)_self;
if (self->exports == 0) {
if (get_exports(self) == 0) {
_memory_release(self);
Py_CLEAR(self->mbuf);
}
Expand Down Expand Up @@ -1589,7 +1600,11 @@ memory_getbuf(PyObject *_self, Py_buffer *view, int flags)


view->obj = Py_NewRef(self);
#ifdef Py_GIL_DISABLED
_Py_atomic_add_ssize(&self->exports, 1);
#else
self->exports++;
#endif

return 0;
}
Expand All @@ -1598,7 +1613,11 @@ static void
memory_releasebuf(PyObject *_self, Py_buffer *view)
{
PyMemoryViewObject *self = (PyMemoryViewObject *)_self;
#ifdef Py_GIL_DISABLED
_Py_atomic_add_ssize(&self->exports, -1);
#else
self->exports--;
#endif
return;
/* PyBuffer_Release() decrements view->obj after this function returns. */
}
Expand Down
Loading