From 8fa2daf18da79308d5321513e87a1bf7bb2032c7 Mon Sep 17 00:00:00 2001 From: Lu Xiaowei Date: Mon, 27 Jul 2026 20:28:10 +0800 Subject: [PATCH 1/5] gh-127716: make memoryview thread-safe Co-authored-by: ayaangazali Co-authored-by: Lu Xiaowei --- Doc/library/threadsafety.rst | 6 + Include/cpython/memoryobject.h | 2 +- .../test_free_threading/test_memoryview.py | 313 +++++++++ Lib/test/test_memoryview.py | 7 + ...-08-21-14-37-50.gh-issue-127716.KrPth8.rst | 9 + Objects/memoryobject.c | 637 +++++++++++++----- 6 files changed, 817 insertions(+), 157 deletions(-) create mode 100644 Lib/test/test_free_threading/test_memoryview.py create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2026-08-21-14-37-50.gh-issue-127716.KrPth8.rst diff --git a/Doc/library/threadsafety.rst b/Doc/library/threadsafety.rst index a529f7803affbc..2ff44b0fabd608 100644 --- a/Doc/library/threadsafety.rst +++ b/Doc/library/threadsafety.rst @@ -604,3 +604,9 @@ underlying object supports it: Resizing or reallocating the underlying object (such as calling :meth:`bytearray.resize`) while a memoryview is exported raises :exc:`BufferError`. This is enforced regardless of threading. + +In the :term:`free-threaded build`, :meth:`~memoryview.release` is +thread-safe. If another thread has an in-flight operation on the memoryview +when it is released, the underlying buffer may remain exported until that +operation completes. Restrictions on the exporter, such as prohibiting +resizing, remain in effect until then. diff --git a/Include/cpython/memoryobject.h b/Include/cpython/memoryobject.h index 961161b70f2058..419e57d1ad56a4 100644 --- a/Include/cpython/memoryobject.h +++ b/Include/cpython/memoryobject.h @@ -11,7 +11,7 @@ typedef struct { PyObject_HEAD int flags; /* state flags */ - Py_ssize_t exports; /* number of direct memoryview exports */ + Py_ssize_t exports; /* registered views and active operations */ Py_buffer master; /* snapshot buffer obtained from the original exporter */ } _PyManagedBufferObject; diff --git a/Lib/test/test_free_threading/test_memoryview.py b/Lib/test/test_free_threading/test_memoryview.py new file mode 100644 index 00000000000000..8493f425d7b86b --- /dev/null +++ b/Lib/test/test_free_threading/test_memoryview.py @@ -0,0 +1,313 @@ +import pickle +import threading +import unittest + +from test.support import threading_helper + +NTHREADS = 8 + +# Fresh objects expose one-time races under ThreadSanitizer. +ROUNDS = 20 +ITERS = 20 + +HASH_DATA_TEMPLATE = bytes(range(256)) +HASH_DATA_REPEAT = 256 + +READ_OPS = [ + lambda mv: mv.tobytes(), + lambda mv: mv.hex(), + lambda mv: mv.tolist(), + lambda mv: mv.cast("B"), + lambda mv: mv.toreadonly(), + lambda mv: memoryview(mv), + lambda mv: bytes(mv), + lambda mv: pickle.PickleBuffer(mv).release(), + lambda mv: mv.obj, + lambda mv: mv.format, + lambda mv: mv.c_contiguous, + lambda mv: mv[0], + lambda mv: mv[0:4], + lambda mv: mv.count(0), + lambda mv: mv.index(0), + lambda mv: mv == mv, + lambda mv: list(mv), + lambda mv: len(mv), +] + + +def run_racy(func, *args): + try: + func(*args) + except (ValueError, BufferError): + pass + + +@threading_helper.requires_working_threading() +class TestMemoryViewRaces(unittest.TestCase): + def assert_exporter_free(self, buf): + buf.append(0) + del buf[-1] + + def test_concurrent_slicing_keeps_export_count(self): + for _ in range(ROUNDS): + mv = memoryview(bytes(64)) + slices = [] + lock = threading.Lock() + + def make_slices(): + local = [mv[0:4] for _ in range(ITERS)] + local += [memoryview(mv) for _ in range(ITERS)] + with lock: + slices.extend(local) + + threading_helper.run_concurrently(make_slices, nthreads=NTHREADS) + del slices + + self.assertEqual(bytes(mv[0:4]), b"\x00" * 4) + + def test_concurrent_release(self): + buf = bytearray(64) + + for _ in range(ROUNDS): + views = [memoryview(buf) for _ in range(NTHREADS)] + + def release(views=views): + for mv in views: + run_racy(mv.release) + + threading_helper.run_concurrently(release, nthreads=NTHREADS) + + self.assert_exporter_free(buf) + + def test_release_races_with_reads(self): + for _ in range(ROUNDS): + buf = bytearray(64) + cell = [memoryview(buf)] + lock = threading.Lock() + + def releaser(): + for _ in range(ITERS): + mv = cell[0] + run_racy(mv.release) + with lock: + cell[0] = memoryview(buf) + + def reader(): + for _ in range(ITERS): + mv = cell[0] + for op in READ_OPS: + run_racy(op, mv) + + threading_helper.run_concurrently( + [releaser] * (NTHREADS // 2) + [reader] * (NTHREADS // 2), + nthreads=NTHREADS, + ) + + cell[0].release() + self.assert_exporter_free(buf) + + def test_read_keeps_exporter_alive_after_release(self): + size = 1 << 20 + for _ in range(ROUNDS): + exporter = [bytearray(size)] + view = memoryview(exporter[0]) + stale = [] + + def reader(): + try: + for _ in range(4): + data = view.tobytes() + if data.count(0) != len(data): + stale.append(True) + return + except ValueError: + pass + + def releaser(): + view.release() + exporter.clear() + for _ in range(8): + bytearray(b"\xdb" * size) + + threading_helper.run_concurrently([reader, releaser]) + self.assertFalse(stale) + + def test_release_races_with_writes(self): + for _ in range(ROUNDS): + buf = bytearray(8 * NTHREADS) + cell = [memoryview(buf)] + lock = threading.Lock() + + def releaser(): + for _ in range(ITERS): + mv = cell[0] + run_racy(mv.release) + with lock: + cell[0] = memoryview(buf) + + def writer(slot): + start = slot * 8 + for _ in range(ITERS): + mv = cell[0] + run_racy(mv.__setitem__, start, 1) + run_racy(mv.__setitem__, slice(start, start + 4), b"abcd") + + workers = [releaser] * (NTHREADS // 2) + workers += [lambda s=s: writer(s) for s in range(NTHREADS // 2)] + threading_helper.run_concurrently(workers, nthreads=NTHREADS) + + cell[0].release() + self.assert_exporter_free(buf) + + def test_release_races_with_buffer_exports(self): + for _ in range(ROUNDS): + buf = bytearray(64) + mv = memoryview(buf) + + def exporter(): + for _ in range(ITERS): + run_racy(lambda: pickle.PickleBuffer(mv).release()) + + def releaser(): + for _ in range(ITERS): + run_racy(mv.release) + + threading_helper.run_concurrently( + [exporter] * (NTHREADS - 1) + [releaser], nthreads=NTHREADS + ) + + mv.release() + self.assert_exporter_free(buf) + + def test_release_with_live_export(self): + buf = bytearray(64) + + for _ in range(ROUNDS): + mv = memoryview(buf) + held = pickle.PickleBuffer(mv) + + def release(mv=mv): + try: + mv.release() + except BufferError: + pass + + threading_helper.run_concurrently(release, nthreads=NTHREADS) + + self.assertEqual(bytes(mv[0:4]), b"\x00" * 4) + held.release() + mv.release() + + self.assert_exporter_free(buf) + + def test_compare_two_views_races_with_release(self): + for _ in range(ROUNDS): + buf = bytearray(64) + cell = [memoryview(buf), memoryview(buf)] + lock = threading.Lock() + + def releaser(slot): + for _ in range(ITERS): + mv = cell[slot] + run_racy(mv.release) + with lock: + cell[slot] = memoryview(buf) + + def comparer(): + for _ in range(ITERS): + left, right = cell[0], cell[1] + run_racy(lambda: left == right) + run_racy(lambda: left != right) + + threading_helper.run_concurrently( + [lambda: releaser(0), lambda: releaser(1)] + [comparer] * 6, + nthreads=NTHREADS, + ) + + for mv in cell: + mv.release() + self.assert_exporter_free(buf) + + def test_release_parent_keeps_child_valid(self): + for _ in range(ROUNDS): + buf = bytearray(range(64)) + parent = memoryview(buf) + child = parent[0:32] + + def use_child(): + for _ in range(ITERS): + self.assertEqual(child[0], 0) + child.tobytes() + + def release_parent(): + parent.release() + + threading_helper.run_concurrently( + [release_parent] + [use_child] * (NTHREADS - 1), + nthreads=NTHREADS, + ) + + self.assertEqual(child.tobytes(), bytes(range(32))) + child.release() + self.assert_exporter_free(buf) + + def test_concurrent_iteration(self): + for _ in range(ROUNDS): + buf = bytearray(range(64)) + cell = [memoryview(buf)] + lock = threading.Lock() + + def releaser(): + for _ in range(ITERS): + mv = cell[0] + run_racy(mv.release) + with lock: + cell[0] = memoryview(buf) + + def iterator(): + for _ in range(ITERS): + run_racy(list, cell[0]) + + threading_helper.run_concurrently( + [releaser] * (NTHREADS // 2) + [iterator] * (NTHREADS // 2), + nthreads=NTHREADS, + ) + + cell[0].release() + self.assert_exporter_free(buf) + + def test_iterator_exhaustion_drops_last_reference(self): + def loop(): + for _ in range(ROUNDS * ITERS): + self.assertEqual(list(iter(memoryview(b"ab"))), [97, 98]) + + threading_helper.run_concurrently(loop, nthreads=NTHREADS) + + def test_concurrent_hash(self): + for _ in range(ROUNDS): + data = HASH_DATA_TEMPLATE * HASH_DATA_REPEAT + mv = memoryview(data) + results = [] + lock = threading.Lock() + + def hasher(): + local = {hash(mv) for _ in range(ITERS)} + with lock: + results.append(local) + + threading_helper.run_concurrently(hasher, nthreads=NTHREADS) + self.assertEqual({h for s in results for h in s}, {hash(data)}) + + def test_concurrent_hash_and_release(self): + for _ in range(ROUNDS): + mv = memoryview(HASH_DATA_TEMPLATE * HASH_DATA_REPEAT) + + def work(mv=mv): + run_racy(hash, mv) + run_racy(mv.release) + + threading_helper.run_concurrently(work, nthreads=NTHREADS) + + +if __name__ == "__main__": + unittest.main() diff --git a/Lib/test/test_memoryview.py b/Lib/test/test_memoryview.py index 3cb8a104faee5e..172b0374cd15f9 100644 --- a/Lib/test/test_memoryview.py +++ b/Lib/test/test_memoryview.py @@ -91,6 +91,13 @@ def test_iter(self): m = self._view(b) self.assertEqual(list(m), [m[i] for i in range(len(m))]) + def test_released_empty_iterator(self): + for tp in self._types: + m = self._view(tp(b"")) + iterator = iter(m) + m.release() + self.assertRaises(StopIteration, next, iterator) + def test_count(self): for tp in self._types: b = tp(self._source) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2026-08-21-14-37-50.gh-issue-127716.KrPth8.rst b/Misc/NEWS.d/next/Core_and_Builtins/2026-08-21-14-37-50.gh-issue-127716.KrPth8.rst new file mode 100644 index 00000000000000..731dd2bb83851c --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2026-08-21-14-37-50.gh-issue-127716.KrPth8.rst @@ -0,0 +1,9 @@ +On the free-threaded build, :class:`memoryview` operations are now thread safe: +an operation that reads the buffer holds the managed buffer for its duration, +so a concurrent :meth:`~memoryview.release` can no longer free the buffer under +it; slicing a memoryview from several threads no longer loses buffer +registrations and raises ``ValueError: operation forbidden on released +memoryview object``; and :func:`hash` of a memoryview is no longer a data race. +As a result :meth:`~memoryview.release` may now return while an operation +started by another thread is still running, see +:ref:`thread-safety-memoryview`. diff --git a/Objects/memoryobject.c b/Objects/memoryobject.c index 0bcd0b6596be77..2bd65d36f54f8e 100644 --- a/Objects/memoryobject.c +++ b/Objects/memoryobject.c @@ -103,13 +103,57 @@ _PyManagedBuffer_FromObject(PyObject *base, int flags) return (PyObject *)mbuf; } +static inline int +flags_test_and_set(int *flags, int bit) +{ +#ifdef Py_GIL_DISABLED + int prev = _Py_atomic_load_int_relaxed(flags); + while (!_Py_atomic_compare_exchange_int(flags, &prev, prev | bit)) { + // Compare-exchange updates prev. + } +#else + int prev = *flags; + *flags = prev | bit; +#endif + return (prev & bit) != 0; +} + +static inline int +mv_get_flags(PyMemoryViewObject *mv) +{ + return FT_ATOMIC_LOAD_INT(mv->flags); +} + +static inline int +mv_has_flag(PyMemoryViewObject *mv, int bits) +{ + return (mv_get_flags(mv) & bits) != 0; +} + +static inline int +mv_set_flag(PyMemoryViewObject *mv, int bit) +{ + return flags_test_and_set(&mv->flags, bit); +} + +static inline int +mbuf_has_flag(_PyManagedBufferObject *mbuf, int bits) +{ + return (FT_ATOMIC_LOAD_INT(mbuf->flags) & bits) != 0; +} + +static inline int +mbuf_set_flag(_PyManagedBufferObject *mbuf, int bit) +{ + return flags_test_and_set(&mbuf->flags, bit); +} + static void mbuf_release(_PyManagedBufferObject *self) { - if (self->flags&_Py_MANAGED_BUFFER_RELEASED) + if (mbuf_set_flag(self, _Py_MANAGED_BUFFER_RELEASED)) { return; - - self->flags |= _Py_MANAGED_BUFFER_RELEASED; + } /* PyBuffer_Release() decrements master->obj and sets it to NULL. */ _PyObject_GC_UNTRACK(self); @@ -122,7 +166,7 @@ mbuf_dealloc(PyObject *_self) _PyManagedBufferObject *self = (_PyManagedBufferObject *)_self; assert(self->exports == 0); mbuf_release(self); - if (self->flags&_Py_MANAGED_BUFFER_FREE_FORMAT) + if (mbuf_has_flag(self, _Py_MANAGED_BUFFER_FREE_FORMAT)) PyMem_Free(self->master.format); PyObject_GC_Del(self); } @@ -178,8 +222,8 @@ PyTypeObject _PyManagedBuffer_Type = { /* In the process of breaking reference cycles mbuf_release() can be called before memory_release(). */ #define BASE_INACCESSIBLE(mv) \ - (((PyMemoryViewObject *)mv)->flags&_Py_MEMORYVIEW_RELEASED || \ - ((PyMemoryViewObject *)mv)->mbuf->flags&_Py_MANAGED_BUFFER_RELEASED) + (mv_has_flag(_PyMemoryView_CAST(mv), _Py_MEMORYVIEW_RELEASED) || \ + mbuf_has_flag(_PyMemoryView_CAST(mv)->mbuf, _Py_MANAGED_BUFFER_RELEASED)) #define CHECK_RELEASED(mv) \ if (BASE_INACCESSIBLE(mv)) { \ @@ -196,17 +240,17 @@ PyTypeObject _PyManagedBuffer_Type = { } #define CHECK_RESTRICTED(mv) \ - if (((PyMemoryViewObject *)(mv))->flags & _Py_MEMORYVIEW_RESTRICTED) { \ - PyErr_SetString(PyExc_ValueError, \ - "cannot create new view on restricted memoryview"); \ - return NULL; \ + if (mv_has_flag(_PyMemoryView_CAST(mv), _Py_MEMORYVIEW_RESTRICTED)) { \ + PyErr_SetString(PyExc_ValueError, \ + "cannot create new view on restricted memoryview"); \ + return NULL; \ } #define CHECK_RESTRICTED_INT(mv) \ - if (((PyMemoryViewObject *)(mv))->flags & _Py_MEMORYVIEW_RESTRICTED) { \ - PyErr_SetString(PyExc_ValueError, \ - "cannot create new view on restricted memoryview"); \ - return -1; \ + if (mv_has_flag(_PyMemoryView_CAST(mv), _Py_MEMORYVIEW_RESTRICTED)) { \ + PyErr_SetString(PyExc_ValueError, \ + "cannot create new view on restricted memoryview"); \ + return -1; \ } /* See gh-92888. These macros signal that we need to check the memoryview @@ -699,7 +743,7 @@ mbuf_add_view(_PyManagedBufferObject *mbuf, const Py_buffer *src) init_flags(mv); mv->mbuf = (_PyManagedBufferObject*)Py_NewRef(mbuf); - mbuf->exports++; + FT_ATOMIC_ADD_SSIZE(mbuf->exports, 1); return (PyObject *)mv; } @@ -729,11 +773,76 @@ mbuf_add_incomplete_view(_PyManagedBufferObject *mbuf, const Py_buffer *src, init_shared_values(dest, src); mv->mbuf = (_PyManagedBufferObject*)Py_NewRef(mbuf); - mbuf->exports++; + FT_ATOMIC_ADD_SSIZE(mbuf->exports, 1); return (PyObject *)mv; } +#ifdef Py_GIL_DISABLED +static int +memoryview_add_export(PyMemoryViewObject *self) +{ + int result = 0; + + Py_BEGIN_CRITICAL_SECTION(self); + if (BASE_INACCESSIBLE(self)) { + PyErr_SetString(PyExc_ValueError, + "operation forbidden on released memoryview object"); + result = -1; + } + else { + FT_ATOMIC_ADD_SSIZE(self->exports, 1); + } + Py_END_CRITICAL_SECTION(); + + return result; +} +#endif + +static void +mbuf_drop_export(_PyManagedBufferObject *mbuf) +{ +#ifdef Py_GIL_DISABLED + Py_ssize_t prev_exports = _Py_atomic_add_ssize(&mbuf->exports, -1); +#else + Py_ssize_t prev_exports = mbuf->exports--; +#endif + assert(prev_exports > 0); + if (prev_exports == 1) { + mbuf_release(mbuf); + } +} + +static inline int +memoryview_pin(PyMemoryViewObject *self) +{ +#ifdef Py_GIL_DISABLED + FT_ATOMIC_ADD_SSIZE(self->mbuf->exports, 1); + if (BASE_INACCESSIBLE(self)) { + mbuf_drop_export(self->mbuf); + PyErr_SetString(PyExc_ValueError, + "operation forbidden on released memoryview object"); + return -1; + } + return 0; +#else + if (BASE_INACCESSIBLE(self)) { + PyErr_SetString(PyExc_ValueError, + "operation forbidden on released memoryview object"); + return -1; + } + return 0; +#endif +} + +static inline void +memoryview_unpin(PyMemoryViewObject *self) +{ +#ifdef Py_GIL_DISABLED + mbuf_drop_export(self->mbuf); +#endif +} + /* Expose a raw memory area as a view of contiguous bytes. flags can be PyBUF_READ or PyBUF_WRITE. view->format is set to "B" (unsigned bytes). The memoryview has complete buffer information. */ @@ -804,9 +913,20 @@ PyMemoryView_FromObjectAndFlags(PyObject *v, int flags) if (PyMemoryView_Check(v)) { PyMemoryViewObject *mv = (PyMemoryViewObject *)v; - CHECK_RELEASED(mv); - CHECK_RESTRICTED(mv); - return mbuf_add_view(mv->mbuf, &mv->view); + if (memoryview_pin(mv) < 0) { + return NULL; + } + PyObject *result; + if (mv_has_flag(mv, _Py_MEMORYVIEW_RESTRICTED)) { + PyErr_SetString(PyExc_ValueError, + "cannot create new view on restricted memoryview"); + result = NULL; + } + else { + result = mbuf_add_view(mv->mbuf, &mv->view); + } + memoryview_unpin(mv); + return result; } else if (PyObject_CheckBuffer(v)) { PyObject *ret; @@ -1089,11 +1209,7 @@ PyBuffer_ToContiguous(void *buf, const Py_buffer *src, Py_ssize_t len, char orde 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 + return FT_ATOMIC_LOAD_SSIZE_RELAXED(buf->exports); } @@ -1109,14 +1225,11 @@ static void _memory_release(PyMemoryViewObject *self) { assert(get_exports(self) == 0); - if (self->flags & _Py_MEMORYVIEW_RELEASED) + if (mv_set_flag(self, _Py_MEMORYVIEW_RELEASED)) { return; - - self->flags |= _Py_MEMORYVIEW_RELEASED; - assert(self->mbuf->exports > 0); - if (--self->mbuf->exports == 0) { - mbuf_release(self->mbuf); } + + mbuf_drop_export(self->mbuf); } /*[clinic input] @@ -1129,22 +1242,26 @@ static PyObject * memoryview_release_impl(PyMemoryViewObject *self) /*[clinic end generated code: output=d0b7e3ba95b7fcb9 input=bc71d1d51f4a52f0]*/ { + PyObject *result = NULL; + + Py_BEGIN_CRITICAL_SECTION(self); Py_ssize_t exports = get_exports(self); if (exports == 0) { _memory_release(self); - Py_RETURN_NONE; + result = Py_NewRef(Py_None); } - - if (exports > 0) { + else if (exports > 0) { PyErr_Format(PyExc_BufferError, "memoryview has %zd exported buffer%s", exports, - exports==1 ? "" : "s"); - return NULL; + exports == 1 ? "" : "s"); + } + else { + PyErr_SetString(PyExc_SystemError, + "memoryview: negative export count"); } + Py_END_CRITICAL_SECTION(); - PyErr_SetString(PyExc_SystemError, - "memoryview: negative export count"); - return NULL; + return result; } static void @@ -1468,25 +1585,9 @@ zero_in_shape(PyMemoryViewObject *mv) All casts must result in views that will have the exact byte size of the original input. Otherwise, an error is raised. */ -/*[clinic input] -memoryview.cast - - format: unicode - shape: object = NULL - * - order: int(accept={str}) = 'C' - -Cast a memoryview to a new format or shape. - -With a multidimensional *shape*, *order* selects the result -layout: 'C' for C-contiguous (row-major, the default) or 'F' -for Fortran-contiguous (column-major). -[clinic start generated code]*/ - static PyObject * -memoryview_cast_impl(PyMemoryViewObject *self, PyObject *format, +memoryview_cast_pinned(PyMemoryViewObject *self, PyObject *format, PyObject *shape, int order) -/*[clinic end generated code: output=6410d87141f6bb56 input=4a1a2326c59caeb3]*/ { PyMemoryViewObject *mv = NULL; Py_ssize_t ndim = 1; @@ -1499,8 +1600,9 @@ memoryview_cast_impl(PyMemoryViewObject *self, PyObject *format, return NULL; } - if (!MV_C_CONTIGUOUS(self->flags)) { - if (shape || !MV_F_CONTIGUOUS(self->flags)) { + int self_flags = mv_get_flags(self); + if (!MV_C_CONTIGUOUS(self_flags)) { + if (shape || !MV_F_CONTIGUOUS(self_flags)) { PyErr_SetString(PyExc_TypeError, "memoryview: casts are restricted to contiguous views"); return NULL; @@ -1544,6 +1646,34 @@ memoryview_cast_impl(PyMemoryViewObject *self, PyObject *format, return NULL; } +/*[clinic input] +memoryview.cast + + format: unicode + shape: object = NULL + * + order: int(accept={str}) = 'C' + +Cast a memoryview to a new format or shape. + +With a multidimensional *shape*, *order* selects the result +layout: 'C' for C-contiguous (row-major, the default) or 'F' +for Fortran-contiguous (column-major). +[clinic start generated code]*/ + +static PyObject * +memoryview_cast_impl(PyMemoryViewObject *self, PyObject *format, + PyObject *shape, int order) +/*[clinic end generated code: output=6410d87141f6bb56 input=4a1a2326c59caeb3]*/ +{ + if (memoryview_pin(self) < 0) { + return NULL; + } + PyObject *result = memoryview_cast_pinned(self, format, shape, order); + memoryview_unpin(self); + return result; +} + /*[clinic input] memoryview.toreadonly @@ -1554,6 +1684,9 @@ static PyObject * memoryview_toreadonly_impl(PyMemoryViewObject *self) /*[clinic end generated code: output=2c7e056f04c99e62 input=dc06d20f19ba236f]*/ { + if (memoryview_pin(self) < 0) { + return NULL; + } CHECK_RELEASED(self); CHECK_RESTRICTED(self); /* Even if self is already readonly, we still need to create a new @@ -1563,6 +1696,7 @@ memoryview_toreadonly_impl(PyMemoryViewObject *self) if (self != NULL) { self->view.readonly = 1; }; + memoryview_unpin(self); return (PyObject *) self; } @@ -1576,7 +1710,7 @@ memory_getbuf(PyObject *_self, Py_buffer *view, int flags) { PyMemoryViewObject *self = (PyMemoryViewObject *)_self; Py_buffer *base = &self->view; - int baseflags = self->flags; + int baseflags = mv_get_flags(self); CHECK_RELEASED_INT(self); CHECK_RESTRICTED_INT(self); @@ -1644,10 +1778,16 @@ memory_getbuf(PyObject *_self, Py_buffer *view, int flags) view->shape = NULL; } +#ifdef Py_GIL_DISABLED + /* memoryview_add_export() keeps the buffer alive or fails. */ + if (memoryview_add_export(self) < 0) { + return -1; + } +#else + self->exports++; +#endif view->obj = Py_NewRef(self); - FT_ATOMIC_ADD_SSIZE(self->exports, 1); - return 0; } @@ -2322,15 +2462,8 @@ tolist_rec(PyMemoryViewObject *self, const char *ptr, Py_ssize_t ndim, const Py_ /* Return a list representation of the memoryview. Currently only buffers with native format strings are supported. */ -/*[clinic input] -memoryview.tolist - -Return the data in the buffer as a list of elements. -[clinic start generated code]*/ - static PyObject * -memoryview_tolist_impl(PyMemoryViewObject *self) -/*[clinic end generated code: output=a6cda89214fd5a1b input=21e7d0c1860b211a]*/ +memoryview_tolist_pinned(PyMemoryViewObject *self) { const Py_buffer *view = &self->view; const char *fmt; @@ -2356,23 +2489,26 @@ memoryview_tolist_impl(PyMemoryViewObject *self) } /*[clinic input] -memoryview.tobytes +memoryview.tolist - order: str(accept={str, NoneType}, c_default="NULL") = 'C' +Return the data in the buffer as a list of elements. +[clinic start generated code]*/ -Return the data in the buffer as a byte string. +static PyObject * +memoryview_tolist_impl(PyMemoryViewObject *self) +/*[clinic end generated code: output=a6cda89214fd5a1b input=21e7d0c1860b211a]*/ +{ + if (memoryview_pin(self) < 0) { + return NULL; + } + PyObject *result = memoryview_tolist_pinned(self); + memoryview_unpin(self); + return result; +} -Order can be {'C', 'F', 'A'}. When order is 'C' or 'F', the data of -the original array is converted to C or Fortran order. For -contiguous views, 'A' returns an exact copy of the physical memory. -In particular, in-memory Fortran order is preserved. For -non-contiguous views, the data is converted to C first. order=None -is the same as order='C'. -[clinic start generated code]*/ static PyObject * -memoryview_tobytes_impl(PyMemoryViewObject *self, const char *order) -/*[clinic end generated code: output=1288b62560a32a23 input=119c70aa91791dc8]*/ +memoryview_tobytes_pinned(PyMemoryViewObject *self, const char *order) { Py_buffer *src = VIEW_ADDR(self); char ord = 'C'; @@ -2408,38 +2544,42 @@ memoryview_tobytes_impl(PyMemoryViewObject *self, const char *order) } /*[clinic input] -memoryview.hex +memoryview.tobytes - sep: object = NULL - An optional single character or byte to separate hex bytes. - bytes_per_sep: Py_ssize_t = 1 - How many bytes between separators. Positive values count from - the right, negative values count from the left. + order: str(accept={str, NoneType}, c_default="NULL") = 'C' -Return the data in the buffer as a str of hexadecimal numbers. +Return the data in the buffer as a byte string. -Example: ->>> value = memoryview(b'\xb9\x01\xef') ->>> value.hex() -'b901ef' ->>> value.hex(':') -'b9:01:ef' ->>> value.hex(':', 2) -'b9:01ef' ->>> value.hex(':', -2) -'b901:ef' +Order can be {'C', 'F', 'A'}. When order is 'C' or 'F', the data of +the original array is converted to C or Fortran order. For +contiguous views, 'A' returns an exact copy of the physical memory. +In particular, in-memory Fortran order is preserved. For +non-contiguous views, the data is converted to C first. order=None +is the same as order='C'. [clinic start generated code]*/ static PyObject * -memoryview_hex_impl(PyMemoryViewObject *self, PyObject *sep, +memoryview_tobytes_impl(PyMemoryViewObject *self, const char *order) +/*[clinic end generated code: output=1288b62560a32a23 input=119c70aa91791dc8]*/ +{ + if (memoryview_pin(self) < 0) { + return NULL; + } + PyObject *result = memoryview_tobytes_pinned(self, order); + memoryview_unpin(self); + return result; +} + + +static PyObject * +memoryview_hex_pinned(PyMemoryViewObject *self, PyObject *sep, Py_ssize_t bytes_per_sep) -/*[clinic end generated code: output=c9bb00c7a8e86056 input=3f1c5d08906e3b70]*/ { Py_buffer *src = VIEW_ADDR(self); CHECK_RELEASED(self); - if (MV_C_CONTIGUOUS(self->flags)) { + if (MV_C_CONTIGUOUS(mv_get_flags(self))) { // Prevent 'self' from being freed if computing len(sep) mutates 'self' // in _Py_strhex_with_sep(). // See: https://github.com/python/cpython/issues/143195. @@ -2469,11 +2609,48 @@ memoryview_hex_impl(PyMemoryViewObject *self, PyObject *sep, return ret; } +/*[clinic input] +memoryview.hex + + sep: object = NULL + An optional single character or byte to separate hex bytes. + bytes_per_sep: Py_ssize_t = 1 + How many bytes between separators. Positive values count from + the right, negative values count from the left. + +Return the data in the buffer as a str of hexadecimal numbers. + +Example: +>>> value = memoryview(b'\xb9\x01\xef') +>>> value.hex() +'b901ef' +>>> value.hex(':') +'b9:01:ef' +>>> value.hex(':', 2) +'b9:01ef' +>>> value.hex(':', -2) +'b901:ef' +[clinic start generated code]*/ + +static PyObject * +memoryview_hex_impl(PyMemoryViewObject *self, PyObject *sep, + Py_ssize_t bytes_per_sep) +/*[clinic end generated code: output=c9bb00c7a8e86056 input=3f1c5d08906e3b70]*/ +{ + if (memoryview_pin(self) < 0) { + return NULL; + } + PyObject *result = memoryview_hex_pinned(self, sep, bytes_per_sep); + memoryview_unpin(self); + return result; +} + + static PyObject * memory_repr(PyObject *_self) { PyMemoryViewObject *self = (PyMemoryViewObject *)_self; - if (self->flags & _Py_MEMORYVIEW_RELEASED) + if (mv_has_flag(self, _Py_MEMORYVIEW_RELEASED)) return PyUnicode_FromFormat("", self); else return PyUnicode_FromFormat("", self); @@ -2546,9 +2723,9 @@ ptr_from_tuple(const Py_buffer *view, PyObject *tup) /* Return the item at index. In a one-dimensional view, this is an object with the type specified by view->format. Otherwise, the item is a sub-view. - The function is used in memory_subscript() and memory_as_sequence. */ + The function is used in memory_subscript_pinned() and by sq_item. */ static PyObject * -memory_item(PyObject *_self, Py_ssize_t index) +memory_item_pinned(PyObject *_self, Py_ssize_t index) { PyMemoryViewObject *self = (PyMemoryViewObject *)_self; Py_buffer *view = &(self->view); @@ -2674,7 +2851,7 @@ is_multiindex(PyObject *key) 0-d memoryview objects can be referenced using mv[...] or mv[()] but not with anything else. */ static PyObject * -memory_subscript(PyObject *_self, PyObject *key) +memory_subscript_pinned(PyObject *_self, PyObject *key) { PyMemoryViewObject *self = (PyMemoryViewObject *)_self; Py_buffer *view; @@ -2704,7 +2881,7 @@ memory_subscript(PyObject *_self, PyObject *key) index = PyNumber_AsSsize_t(key, PyExc_IndexError); if (index == -1 && PyErr_Occurred()) return NULL; - return memory_item((PyObject *)self, index); + return memory_item_pinned((PyObject *)self, index); } else if (PySlice_Check(key)) { CHECK_RESTRICTED(self); @@ -2737,7 +2914,7 @@ memory_subscript(PyObject *_self, PyObject *key) } static int -memory_ass_sub(PyObject *_self, PyObject *key, PyObject *value) +memory_ass_sub_pinned(PyObject *_self, PyObject *key, PyObject *value) { PyMemoryViewObject *self = (PyMemoryViewObject *)_self; Py_buffer *view = &(self->view); @@ -2827,7 +3004,7 @@ memory_ass_sub(PyObject *_self, PyObject *key, PyObject *value) return pack_single(self, ptr, value, fmt); } if (PySlice_Check(key) || is_multislice(key)) { - /* Call memory_subscript() to produce a sliced lvalue, then copy + /* Call memory_subscript_pinned() to produce a sliced lvalue, then copy rvalue into lvalue. This is already implemented in _testbuffer.c. */ PyErr_SetString(PyExc_NotImplementedError, "memoryview slice assignments are currently restricted " @@ -2851,6 +3028,42 @@ memory_length(PyObject *_self) return self->view.shape[0]; } +static PyObject * +memory_item(PyObject *_self, Py_ssize_t index) +{ + PyMemoryViewObject *self = (PyMemoryViewObject *)_self; + if (memoryview_pin(self) < 0) { + return NULL; + } + PyObject *result = memory_item_pinned(_self, index); + memoryview_unpin(self); + return result; +} + +static PyObject * +memory_subscript(PyObject *_self, PyObject *key) +{ + PyMemoryViewObject *self = (PyMemoryViewObject *)_self; + if (memoryview_pin(self) < 0) { + return NULL; + } + PyObject *result = memory_subscript_pinned(_self, key); + memoryview_unpin(self); + return result; +} + +static int +memory_ass_sub(PyObject *_self, PyObject *key, PyObject *value) +{ + PyMemoryViewObject *self = (PyMemoryViewObject *)_self; + if (memoryview_pin(self) < 0) { + return -1; + } + int result = memory_ass_sub_pinned(_self, key, value); + memoryview_unpin(self); + return result; +} + /* As mapping */ static PyMappingMethods memory_as_mapping = { memory_length, /* mp_length */ @@ -2871,18 +3084,8 @@ static PySequenceMethods memory_as_sequence = { /* Counting */ /****************************************************************************/ -/*[clinic input] -memoryview.count - - value: object - / - -Count the number of occurrences of a value. -[clinic start generated code]*/ - static PyObject * -memoryview_count_impl(PyMemoryViewObject *self, PyObject *value) -/*[clinic end generated code: output=a15cb19311985063 input=e3036ce1ed7d1823]*/ +memoryview_count_pinned(PyMemoryViewObject *self, PyObject *value) { PyObject *iter = PyObject_GetIter(_PyObject_CAST(self)); if (iter == NULL) { @@ -2915,28 +3118,36 @@ memoryview_count_impl(PyMemoryViewObject *self, PyObject *value) return PyLong_FromSsize_t(count); } - -/**************************************************************************/ -/* Lookup */ -/**************************************************************************/ - /*[clinic input] -memoryview.index +memoryview.count value: object - start: slice_index(accept={int}) = 0 - stop: slice_index(accept={int}, c_default="PY_SSIZE_T_MAX") = sys.maxsize / -Return the index of the first occurrence of a value. - -Raises ValueError if the value is not present. +Count the number of occurrences of a value. [clinic start generated code]*/ static PyObject * -memoryview_index_impl(PyMemoryViewObject *self, PyObject *value, - Py_ssize_t start, Py_ssize_t stop) -/*[clinic end generated code: output=e0185e3819e549df input=0697a0165bf90b5a]*/ +memoryview_count_impl(PyMemoryViewObject *self, PyObject *value) +/*[clinic end generated code: output=a15cb19311985063 input=e3036ce1ed7d1823]*/ +{ + if (memoryview_pin(self) < 0) { + return NULL; + } + PyObject *result = memoryview_count_pinned(self, value); + memoryview_unpin(self); + return result; +} + + + +/**************************************************************************/ +/* Lookup */ +/**************************************************************************/ + +static PyObject * +memoryview_index_pinned(PyMemoryViewObject *self, PyObject *value, + Py_ssize_t start, Py_ssize_t stop) { const Py_buffer *view = &self->view; CHECK_RELEASED(self); @@ -2973,7 +3184,7 @@ memoryview_index_impl(PyMemoryViewObject *self, PyObject *value, // entire loop. assert(index < n); - PyObject *item = memory_item(obj, index); + PyObject *item = memory_item_pinned(obj, index); if (item == NULL) { return NULL; } @@ -3001,6 +3212,32 @@ memoryview_index_impl(PyMemoryViewObject *self, PyObject *value, } +/*[clinic input] +memoryview.index + + value: object + start: slice_index(accept={int}) = 0 + stop: slice_index(accept={int}, c_default="PY_SSIZE_T_MAX") = sys.maxsize + / + +Return the index of the first occurrence of a value. + +Raises ValueError if the value is not present. +[clinic start generated code]*/ + +static PyObject * +memoryview_index_impl(PyMemoryViewObject *self, PyObject *value, + Py_ssize_t start, Py_ssize_t stop) +/*[clinic end generated code: output=e0185e3819e549df input=0697a0165bf90b5a]*/ +{ + if (memoryview_pin(self) < 0) { + return NULL; + } + PyObject *result = memoryview_index_pinned(self, value, start, stop); + memoryview_unpin(self); + return result; +} + /**************************************************************************/ /* Comparisons */ @@ -3216,7 +3453,7 @@ cmp_rec(const char *p, const char *q, } static PyObject * -memory_richcompare(PyObject *v, PyObject *w, int op) +memory_richcompare_pinned(PyObject *v, PyObject *w, int op) { PyObject *res; Py_buffer wbuf, *vv; @@ -3342,15 +3579,44 @@ memory_richcompare(PyObject *v, PyObject *w, int op) return Py_XNewRef(res); } +static PyObject * +memory_richcompare(PyObject *v, PyObject *w, int op) +{ + PyMemoryViewObject *self = (PyMemoryViewObject *)v; + if (memoryview_pin(self) < 0) { + /* Released memoryviews still compare by identity. */ + PyErr_Clear(); + return memory_richcompare_pinned(v, w, op); + } + + PyMemoryViewObject *other = NULL; + if (PyMemoryView_Check(w) && w != v) { + other = (PyMemoryViewObject *)w; + if (memoryview_pin(other) < 0) { + PyErr_Clear(); + memoryview_unpin(self); + return memory_richcompare_pinned(v, w, op); + } + } + + PyObject *result = memory_richcompare_pinned(v, w, op); + if (other != NULL) { + memoryview_unpin(other); + } + memoryview_unpin(self); + return result; +} + /**************************************************************************/ /* Hash */ /**************************************************************************/ static Py_hash_t -memory_hash(PyObject *_self) +memory_hash_pinned(PyObject *_self) { PyMemoryViewObject *self = (PyMemoryViewObject *)_self; - if (self->hash == -1) { + Py_hash_t hash = FT_ATOMIC_LOAD_SSIZE_RELAXED(self->hash); + if (hash == -1) { Py_buffer *view = &self->view; char *mem = view->buf; Py_ssize_t ret; @@ -3381,7 +3647,7 @@ memory_hash(PyObject *_self) } } - if (!MV_C_CONTIGUOUS(self->flags)) { + if (!MV_C_CONTIGUOUS(mv_get_flags(self))) { mem = PyMem_Malloc(view->len); if (mem == NULL) { PyErr_NoMemory(); @@ -3394,13 +3660,30 @@ memory_hash(PyObject *_self) } /* Can't fail */ - self->hash = Py_HashBuffer(mem, view->len); + hash = Py_HashBuffer(mem, view->len); + FT_ATOMIC_STORE_SSIZE_RELAXED(self->hash, hash); if (mem != view->buf) PyMem_Free(mem); } - return self->hash; + return hash; +} + +static Py_hash_t +memory_hash(PyObject *_self) +{ + PyMemoryViewObject *self = (PyMemoryViewObject *)_self; + Py_hash_t hash = FT_ATOMIC_LOAD_SSIZE_RELAXED(self->hash); + if (hash != -1) { + return hash; + } + if (memoryview_pin(self) < 0) { + return -1; + } + Py_hash_t result = memory_hash_pinned(_self); + memoryview_unpin(self); + return result; } @@ -3433,7 +3716,7 @@ _IntTupleFromSsizet(int len, Py_ssize_t *vals) } static PyObject * -memory_obj_get(PyObject *_self, void *Py_UNUSED(ignored)) +memory_obj_get_pinned(PyObject *_self, void *Py_UNUSED(ignored)) { PyMemoryViewObject *self = (PyMemoryViewObject *)_self; Py_buffer *view = &self->view; @@ -3454,7 +3737,7 @@ memory_nbytes_get(PyObject *_self, void *Py_UNUSED(ignored)) } static PyObject * -memory_format_get(PyObject *_self, void *Py_UNUSED(ignored)) +memory_format_get_pinned(PyObject *_self, void *Py_UNUSED(ignored)) { PyMemoryViewObject *self = (PyMemoryViewObject *)_self; CHECK_RELEASED(self); @@ -3514,7 +3797,7 @@ memory_c_contiguous(PyObject *_self, void *Py_UNUSED(ignored)) { PyMemoryViewObject *self = (PyMemoryViewObject *)_self; CHECK_RELEASED(self); - return PyBool_FromLong(MV_C_CONTIGUOUS(self->flags)); + return PyBool_FromLong(MV_C_CONTIGUOUS(mv_get_flags(self))); } static PyObject * @@ -3522,7 +3805,7 @@ memory_f_contiguous(PyObject *_self, void *Py_UNUSED(ignored)) { PyMemoryViewObject *self = (PyMemoryViewObject *)_self; CHECK_RELEASED(self); - return PyBool_FromLong(MV_F_CONTIGUOUS(self->flags)); + return PyBool_FromLong(MV_F_CONTIGUOUS(mv_get_flags(self))); } static PyObject * @@ -3530,9 +3813,27 @@ memory_contiguous(PyObject *_self, void *Py_UNUSED(ignored)) { PyMemoryViewObject *self = (PyMemoryViewObject *)_self; CHECK_RELEASED(self); - return PyBool_FromLong(MV_ANY_CONTIGUOUS(self->flags)); + return PyBool_FromLong(MV_ANY_CONTIGUOUS(mv_get_flags(self))); } +#define MEMORYVIEW_PINNED_GETTER(name) \ + static PyObject * \ + name(PyObject *_self, void *ignored) \ + { \ + PyMemoryViewObject *self = (PyMemoryViewObject *)_self; \ + if (memoryview_pin(self) < 0) { \ + return NULL; \ + } \ + PyObject *result = name##_pinned(_self, ignored); \ + memoryview_unpin(self); \ + return result; \ + } + +MEMORYVIEW_PINNED_GETTER(memory_obj_get) +MEMORYVIEW_PINNED_GETTER(memory_format_get) + +#undef MEMORYVIEW_PINNED_GETTER + PyDoc_STRVAR(memory_obj_doc, "The underlying object of the memoryview."); PyDoc_STRVAR(memory_nbytes_doc, @@ -3636,32 +3937,44 @@ static PyObject * memoryiter_next(PyObject *self) { memoryiterobject *it = (memoryiterobject *)self; - PyMemoryViewObject *seq; - seq = it->it_seq; + PyMemoryViewObject *seq = it->it_seq; if (seq == NULL) { return NULL; } - if (it->it_index < it->it_length) { - CHECK_RELEASED(seq); - Py_buffer *view = &(seq->view); - char *ptr = (char *)seq->view.buf; + if (it->it_index >= it->it_length) { + /* Preserve StopIteration for exhausted iterators over released views. */ + it->it_seq = NULL; + Py_DECREF(seq); + return NULL; + } + +#ifdef Py_GIL_DISABLED + /* Keep seq alive if another thread exhausts the iterator. */ + Py_INCREF(seq); +#endif + + PyObject *result = NULL; + if (memoryview_pin(seq) == 0) { + Py_buffer *view = &seq->view; + char *ptr = (char *)view->buf; ptr += view->strides[0] * it->it_index++; ptr = ADJUST_PTR(ptr, view->suboffsets, 0); - if (ptr == NULL) { - return NULL; + if (ptr != NULL) { + result = unpack_single(seq, ptr, it->it_fmt); } - return unpack_single(seq, ptr, it->it_fmt); + memoryview_unpin(seq); } - it->it_seq = NULL; +#ifdef Py_GIL_DISABLED Py_DECREF(seq); - return NULL; +#endif + return result; } static PyObject * -memory_iter(PyObject *seq) +memory_iter_pinned(PyObject *seq) { if (!PyMemoryView_Check(seq)) { PyErr_BadInternalCall(); @@ -3698,6 +4011,18 @@ memory_iter(PyObject *seq) return (PyObject *)it; } +static PyObject * +memory_iter(PyObject *seq) +{ + PyMemoryViewObject *self = (PyMemoryViewObject *)seq; + if (memoryview_pin(self) < 0) { + return NULL; + } + PyObject *result = memory_iter_pinned(seq); + memoryview_unpin(self); + return result; +} + PyTypeObject _PyMemoryIter_Type = { PyVarObject_HEAD_INIT(&PyType_Type, 0) .tp_name = "memory_iterator", From fb7e67229a12c0473caff346bebd2632a0b1312b Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Tue, 25 Aug 2026 18:55:40 -0700 Subject: [PATCH 2/5] Fix two bugs. * The CHECK_* macros can return. Restructure code so that unpin is always done for toreadonly(). * Use atomic load for asserts on `exports`. --- Objects/memoryobject.c | 47 +++++++++++++++++++++++++++++------------- 1 file changed, 33 insertions(+), 14 deletions(-) diff --git a/Objects/memoryobject.c b/Objects/memoryobject.c index 2bd65d36f54f8e..75c66853d849a1 100644 --- a/Objects/memoryobject.c +++ b/Objects/memoryobject.c @@ -164,7 +164,7 @@ static void mbuf_dealloc(PyObject *_self) { _PyManagedBufferObject *self = (_PyManagedBufferObject *)_self; - assert(self->exports == 0); + assert(FT_ATOMIC_LOAD_SSIZE_RELAXED(self->exports) == 0); mbuf_release(self); if (mbuf_has_flag(self, _Py_MANAGED_BUFFER_FREE_FORMAT)) PyMem_Free(self->master.format); @@ -183,7 +183,7 @@ static int mbuf_clear(PyObject *_self) { _PyManagedBufferObject *self = (_PyManagedBufferObject *)_self; - assert(self->exports >= 0); + assert(FT_ATOMIC_LOAD_SSIZE_RELAXED(self->exports) >= 0); mbuf_release(self); return 0; } @@ -1674,6 +1674,21 @@ memoryview_cast_impl(PyMemoryViewObject *self, PyObject *format, return result; } +static PyObject * +memoryview_toreadonly_pinned(PyMemoryViewObject *self) +{ + CHECK_RELEASED(self); + CHECK_RESTRICTED(self); + /* Even if self is already readonly, we still need to create a new + * object for .release() to work correctly. + */ + self = (PyMemoryViewObject *) mbuf_add_view(self->mbuf, &self->view); + if (self != NULL) { + self->view.readonly = 1; + }; + return (PyObject *) self; +} + /*[clinic input] memoryview.toreadonly @@ -1687,17 +1702,9 @@ memoryview_toreadonly_impl(PyMemoryViewObject *self) if (memoryview_pin(self) < 0) { return NULL; } - CHECK_RELEASED(self); - CHECK_RESTRICTED(self); - /* Even if self is already readonly, we still need to create a new - * object for .release() to work correctly. - */ - self = (PyMemoryViewObject *) mbuf_add_view(self->mbuf, &self->view); - if (self != NULL) { - self->view.readonly = 1; - }; + PyObject *result = memoryview_toreadonly_pinned(self); memoryview_unpin(self); - return (PyObject *) self; + return result; } @@ -2583,7 +2590,13 @@ memoryview_hex_pinned(PyMemoryViewObject *self, PyObject *sep, // Prevent 'self' from being freed if computing len(sep) mutates 'self' // in _Py_strhex_with_sep(). // See: https://github.com/python/cpython/issues/143195. - FT_ATOMIC_ADD_SSIZE(self->exports, 1); +#ifdef Py_GIL_DISABLED + if (memoryview_add_export(self) < 0) { + return NULL; + } +#else + self->exports++; +#endif PyObject *ret = _Py_strhex_with_sep(src->buf, src->len, sep, bytes_per_sep); FT_ATOMIC_ADD_SSIZE(self->exports, -1); return ret; @@ -3638,7 +3651,13 @@ memory_hash_pinned(PyObject *_self) if (view->obj != NULL) { // Prevent 'self' from being freed when computing the item's hash. // See https://github.com/python/cpython/issues/142664. - FT_ATOMIC_ADD_SSIZE(self->exports, 1); +#ifdef Py_GIL_DISABLED + if (memoryview_add_export(self) < 0) { + return -1; + } +#else + self->exports++; +#endif Py_hash_t h = PyObject_Hash(view->obj); FT_ATOMIC_ADD_SSIZE(self->exports, -1); if (h == -1) { From 908a2a1fccf07546a5583dbfb4b05159e98aa97c Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Thu, 27 Aug 2026 11:58:03 -0700 Subject: [PATCH 3/5] Unit test improvements. Speed it up (less iterations, rounds and threads). Rename confusing assert_exporter_free() method to ensure_exporter_free(). --- .../test_free_threading/test_memoryview.py | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/Lib/test/test_free_threading/test_memoryview.py b/Lib/test/test_free_threading/test_memoryview.py index 8493f425d7b86b..b8369e912d93bb 100644 --- a/Lib/test/test_free_threading/test_memoryview.py +++ b/Lib/test/test_free_threading/test_memoryview.py @@ -4,11 +4,9 @@ from test.support import threading_helper -NTHREADS = 8 - -# Fresh objects expose one-time races under ThreadSanitizer. -ROUNDS = 20 -ITERS = 20 +NTHREADS = 4 +ROUNDS = 4 +ITERS = 10 HASH_DATA_TEMPLATE = bytes(range(256)) HASH_DATA_REPEAT = 256 @@ -44,7 +42,8 @@ def run_racy(func, *args): @threading_helper.requires_working_threading() class TestMemoryViewRaces(unittest.TestCase): - def assert_exporter_free(self, buf): + def ensure_exporter_free(self, buf): + # Will raise BufferError if any memoryview still exports it. buf.append(0) del buf[-1] @@ -77,7 +76,7 @@ def release(views=views): threading_helper.run_concurrently(release, nthreads=NTHREADS) - self.assert_exporter_free(buf) + self.ensure_exporter_free(buf) def test_release_races_with_reads(self): for _ in range(ROUNDS): @@ -104,7 +103,7 @@ def reader(): ) cell[0].release() - self.assert_exporter_free(buf) + self.ensure_exporter_free(buf) def test_read_keeps_exporter_alive_after_release(self): size = 1 << 20 @@ -157,7 +156,7 @@ def writer(slot): threading_helper.run_concurrently(workers, nthreads=NTHREADS) cell[0].release() - self.assert_exporter_free(buf) + self.ensure_exporter_free(buf) def test_release_races_with_buffer_exports(self): for _ in range(ROUNDS): @@ -177,7 +176,7 @@ def releaser(): ) mv.release() - self.assert_exporter_free(buf) + self.ensure_exporter_free(buf) def test_release_with_live_export(self): buf = bytearray(64) @@ -198,7 +197,7 @@ def release(mv=mv): held.release() mv.release() - self.assert_exporter_free(buf) + self.ensure_exporter_free(buf) def test_compare_two_views_races_with_release(self): for _ in range(ROUNDS): @@ -220,13 +219,14 @@ def comparer(): run_racy(lambda: left != right) threading_helper.run_concurrently( - [lambda: releaser(0), lambda: releaser(1)] + [comparer] * 6, + [lambda: releaser(0), lambda: releaser(1)] + + [comparer] * (NTHREADS - 2), nthreads=NTHREADS, ) for mv in cell: mv.release() - self.assert_exporter_free(buf) + self.ensure_exporter_free(buf) def test_release_parent_keeps_child_valid(self): for _ in range(ROUNDS): @@ -249,7 +249,7 @@ def release_parent(): self.assertEqual(child.tobytes(), bytes(range(32))) child.release() - self.assert_exporter_free(buf) + self.ensure_exporter_free(buf) def test_concurrent_iteration(self): for _ in range(ROUNDS): @@ -274,7 +274,7 @@ def iterator(): ) cell[0].release() - self.assert_exporter_free(buf) + self.ensure_exporter_free(buf) def test_iterator_exhaustion_drops_last_reference(self): def loop(): From 6692fa52c1f3b16d3d3197247549f82df5de388a Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Fri, 28 Aug 2026 08:43:06 -0700 Subject: [PATCH 4/5] Address review feedback. * Add comment to CAS while loop, yielding is unnecessary. * Use "relaxed" memory ordering, seq-cst is overkill. --- Objects/memoryobject.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Objects/memoryobject.c b/Objects/memoryobject.c index 75c66853d849a1..1628a1b0432398 100644 --- a/Objects/memoryobject.c +++ b/Objects/memoryobject.c @@ -109,7 +109,9 @@ flags_test_and_set(int *flags, int bit) #ifdef Py_GIL_DISABLED int prev = _Py_atomic_load_int_relaxed(flags); while (!_Py_atomic_compare_exchange_int(flags, &prev, prev | bit)) { - // Compare-exchange updates prev. + // Compare-exchange updates prev. Note that retries are bounded by the + // number of flag bits being concurrently set. So, yielding here is + // unnecessary. } #else int prev = *flags; @@ -121,7 +123,7 @@ flags_test_and_set(int *flags, int bit) static inline int mv_get_flags(PyMemoryViewObject *mv) { - return FT_ATOMIC_LOAD_INT(mv->flags); + return FT_ATOMIC_LOAD_INT_RELAXED(mv->flags); } static inline int From e3c9012a878f5cc98279604229ba45104bf46833 Mon Sep 17 00:00:00 2001 From: Neil Schemenauer Date: Fri, 28 Aug 2026 09:37:31 -0700 Subject: [PATCH 5/5] Remove "while" loop for setting flags. We only need the "while" loop if we are setting multiple flag bits concurrently. That's not needed, we only set RELEASED. So, restructure code, no while loop needed and someone can't accidently call the function in the wrong way. --- Objects/memoryobject.c | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/Objects/memoryobject.c b/Objects/memoryobject.c index 1628a1b0432398..2d8537ee5721c8 100644 --- a/Objects/memoryobject.c +++ b/Objects/memoryobject.c @@ -104,20 +104,24 @@ _PyManagedBuffer_FromObject(PyObject *base, int flags) } static inline int -flags_test_and_set(int *flags, int bit) +flags_set_released(int *flags, int released) { #ifdef Py_GIL_DISABLED int prev = _Py_atomic_load_int_relaxed(flags); - while (!_Py_atomic_compare_exchange_int(flags, &prev, prev | bit)) { - // Compare-exchange updates prev. Note that retries are bounded by the - // number of flag bits being concurrently set. So, yielding here is - // unnecessary. + if (prev & released) { + return 1; } + if (!_Py_atomic_compare_exchange_int(flags, &prev, prev | released)) { + // RELEASED is the only flag that can change concurrently. + assert(prev & released); + return 1; + } + return 0; #else int prev = *flags; - *flags = prev | bit; + *flags = prev | released; + return (prev & released) != 0; #endif - return (prev & bit) != 0; } static inline int @@ -133,9 +137,9 @@ mv_has_flag(PyMemoryViewObject *mv, int bits) } static inline int -mv_set_flag(PyMemoryViewObject *mv, int bit) +mv_set_released(PyMemoryViewObject *mv) { - return flags_test_and_set(&mv->flags, bit); + return flags_set_released(&mv->flags, _Py_MEMORYVIEW_RELEASED); } static inline int @@ -145,15 +149,15 @@ mbuf_has_flag(_PyManagedBufferObject *mbuf, int bits) } static inline int -mbuf_set_flag(_PyManagedBufferObject *mbuf, int bit) +mbuf_set_released(_PyManagedBufferObject *mbuf) { - return flags_test_and_set(&mbuf->flags, bit); + return flags_set_released(&mbuf->flags, _Py_MANAGED_BUFFER_RELEASED); } static void mbuf_release(_PyManagedBufferObject *self) { - if (mbuf_set_flag(self, _Py_MANAGED_BUFFER_RELEASED)) { + if (mbuf_set_released(self)) { return; } @@ -1227,7 +1231,7 @@ static void _memory_release(PyMemoryViewObject *self) { assert(get_exports(self) == 0); - if (mv_set_flag(self, _Py_MEMORYVIEW_RELEASED)) { + if (mv_set_released(self)) { return; }