]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
gh-127085: fix some data races in memoryview in free-threading (#127412)
authorEdward Xu <xuxiangad@foxmail.com>
Mon, 16 Dec 2024 19:12:19 +0000 (03:12 +0800)
committerGitHub <noreply@github.com>
Mon, 16 Dec 2024 19:12:19 +0000 (00:42 +0530)
Lib/test/test_memoryview.py
Misc/NEWS.d/next/Core_and_Builtins/2024-11-30-23-35-45.gh-issue-127085.KLKylb.rst [new file with mode: 0644]
Objects/memoryobject.c

index 96320ebef6467aa7675923182198755683a001b6..856e17918048e458eae495f44182f2eb782ea9d5 100644 (file)
@@ -16,7 +16,8 @@ import pickle
 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:
@@ -733,5 +734,28 @@ class OtherTest(unittest.TestCase):
         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()
diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-11-30-23-35-45.gh-issue-127085.KLKylb.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-11-30-23-35-45.gh-issue-127085.KLKylb.rst
new file mode 100644 (file)
index 0000000..a59b950
--- /dev/null
@@ -0,0 +1 @@
+Fix race when exporting a buffer from a :class:`memoryview` object on the :term:`free-threaded <free threading>` build.
index afe2dcb127adb4ad8c81bf570ff40a8c9f356893..ea4d24dc69076877f33f35023e7c8469ab90018c 100644 (file)
@@ -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                          */
@@ -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;
 
@@ -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;
     }
 
@@ -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);
@@ -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);
     }
@@ -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;
 }
@@ -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. */
 }