From b7fb7a47071bb85c054576d6e0cb17b6622505e2 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Fri, 5 May 2023 21:39:58 -0700 Subject: [PATCH 01/11] Fix buffer classes using super() --- Include/internal/pycore_memoryobject.h | 3 ++- Lib/test/test_buffer.py | 9 +++++++++ Objects/bytearrayobject.c | 1 + Objects/memoryobject.c | 26 +++++++++++++++++++++++++- Objects/typeobject.c | 11 ++++++++--- 5 files changed, 45 insertions(+), 5 deletions(-) diff --git a/Include/internal/pycore_memoryobject.h b/Include/internal/pycore_memoryobject.h index acc12c9275172c..fe19e3f9611a16 100644 --- a/Include/internal/pycore_memoryobject.h +++ b/Include/internal/pycore_memoryobject.h @@ -9,7 +9,8 @@ extern "C" { #endif PyObject * -PyMemoryView_FromObjectAndFlags(PyObject *v, int flags); +_PyMemoryView_FromBufferProc(PyObject *v, int flags, + getbufferproc bufferproc); #ifdef __cplusplus } diff --git a/Lib/test/test_buffer.py b/Lib/test/test_buffer.py index b6e82ad4db266a..3469eefa875130 100644 --- a/Lib/test/test_buffer.py +++ b/Lib/test/test_buffer.py @@ -4579,6 +4579,15 @@ def test_c_buffer(self): buf.__release_buffer__(mv) self.assertEqual(buf.references, 0) + def test_inheritance(self): + class A(bytearray): + def __buffer__(self, flags): + return super().__buffer__(flags) + + a = A(b"hello") + mv = memoryview(a) + self.assertEqual(mv.tobytes(), b"hello") + if __name__ == "__main__": unittest.main() diff --git a/Objects/bytearrayobject.c b/Objects/bytearrayobject.c index 49d4dd524696a5..c36db59baaa10d 100644 --- a/Objects/bytearrayobject.c +++ b/Objects/bytearrayobject.c @@ -61,6 +61,7 @@ static void bytearray_releasebuffer(PyByteArrayObject *obj, Py_buffer *view) { obj->ob_exports--; + assert(obj->ob_exports >= 0); } static int diff --git a/Objects/memoryobject.c b/Objects/memoryobject.c index f008a8cc3e0474..0992a70fade448 100644 --- a/Objects/memoryobject.c +++ b/Objects/memoryobject.c @@ -781,7 +781,7 @@ PyMemoryView_FromBuffer(const Py_buffer *info) using the given flags. If the object is a memoryview, the new memoryview must be registered with the same managed buffer. Otherwise, a new managed buffer is created. */ -PyObject * +static PyObject * PyMemoryView_FromObjectAndFlags(PyObject *v, int flags) { _PyManagedBufferObject *mbuf; @@ -806,6 +806,30 @@ PyMemoryView_FromObjectAndFlags(PyObject *v, int flags) Py_TYPE(v)->tp_name); return NULL; } + +/* Create a memoryview from an object that implements the buffer protocol, + using the given flags. + If the object is a memoryview, the new memoryview must be registered + with the same managed buffer. Otherwise, a new managed buffer is created. */ +PyObject * +_PyMemoryView_FromBufferProc(PyObject *v, int flags, getbufferproc bufferproc) +{ + _PyManagedBufferObject *mbuf = mbuf_alloc(); + if (mbuf == NULL) + return NULL; + + int res = bufferproc(v, &mbuf->master, flags); + if (res < 0) { + mbuf->master.obj = NULL; + Py_DECREF(mbuf); + return NULL; + } + + PyObject *ret = mbuf_add_view(mbuf, NULL); + Py_DECREF(mbuf); + return ret; +} + /* Create a memoryview from an object that implements the buffer protocol. If the object is a memoryview, the new memoryview must be registered with the same managed buffer. Otherwise, a new managed buffer is created. */ diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 456b10ee01d6bc..1e8abbe92844ea 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -6,7 +6,7 @@ #include "pycore_symtable.h" // _Py_Mangle() #include "pycore_dict.h" // _PyDict_KeysSize() #include "pycore_initconfig.h" // _PyStatus_OK() -#include "pycore_memoryobject.h" // PyMemoryView_FromObjectAndFlags() +#include "pycore_memoryobject.h" // _PyMemoryView_FromBufferProc() #include "pycore_moduleobject.h" // _PyModule_GetDef() #include "pycore_object.h" // _PyType_HasFeature() #include "pycore_long.h" // _PyLong_IsNegative() @@ -56,6 +56,8 @@ typedef struct PySlot_Offset { short slot_offset; } PySlot_Offset; +static void +slot_bf_releasebuffer(PyObject *self, Py_buffer *buffer); static PyObject * slot_tp_new(PyTypeObject *type, PyObject *args, PyObject *kwds); @@ -8078,7 +8080,8 @@ wrap_buffer(PyObject *self, PyObject *args, void *wrapped) return NULL; } - return PyMemoryView_FromObjectAndFlags(self, Py_SAFE_DOWNCAST(flags, Py_ssize_t, int)); + return _PyMemoryView_FromBufferProc(self, Py_SAFE_DOWNCAST(flags, Py_ssize_t, int), + (getbufferproc)wrapped); } static PyObject * @@ -8980,8 +8983,10 @@ bufferwrapper_releasebuf(PyObject *self, Py_buffer *view) assert(PyMemoryView_Check(bw->mv)); Py_TYPE(bw->mv)->tp_as_buffer->bf_releasebuffer(bw->mv, view); + // We only need to call bf_releasebuffer if it's a Python function. If it's a C + // bf_releasebuf, it will be called when the memoryview is released. if (Py_TYPE(bw->obj)->tp_as_buffer != NULL - && Py_TYPE(bw->obj)->tp_as_buffer->bf_releasebuffer != NULL) { + && Py_TYPE(bw->obj)->tp_as_buffer->bf_releasebuffer == slot_bf_releasebuffer) { Py_TYPE(bw->obj)->tp_as_buffer->bf_releasebuffer(bw->obj, view); } } From c717784f82d4eab23f7bd44217553b9989619806 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Sat, 6 May 2023 06:34:30 -0700 Subject: [PATCH 02/11] Fix bf_releasebuffer in child classes --- Lib/test/test_buffer.py | 16 ++++++++ Objects/typeobject.c | 81 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 93 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_buffer.py b/Lib/test/test_buffer.py index 3469eefa875130..44a4fdf7bcc7ca 100644 --- a/Lib/test/test_buffer.py +++ b/Lib/test/test_buffer.py @@ -4588,6 +4588,22 @@ def __buffer__(self, flags): mv = memoryview(a) self.assertEqual(mv.tobytes(), b"hello") + def test_inheritance_releasebuffer(self): + rb_call_count = 0 + class B(bytearray): + def __buffer__(self, flags): + return super().__buffer__(flags) + def __release_buffer__(self, view): + nonlocal rb_call_count + rb_call_count += 1 + super().__release_buffer__(view) + + b = B(b"hello") + with memoryview(b) as mv: + self.assertEqual(mv.tobytes(), b"hello") + self.assertEqual(rb_call_count, 0) + self.assertEqual(rb_call_count, 1) + if __name__ == "__main__": unittest.main() diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 1e8abbe92844ea..ae27aa5ccb3de7 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -59,6 +59,9 @@ typedef struct PySlot_Offset { static void slot_bf_releasebuffer(PyObject *self, Py_buffer *buffer); +static void +releasebuffer_call_python(PyObject *self, Py_buffer *buffer); + static PyObject * slot_tp_new(PyTypeObject *type, PyObject *args, PyObject *kwds); @@ -8097,6 +8100,10 @@ wrap_releasebuffer(PyObject *self, PyObject *args, void *wrapped) return NULL; } PyMemoryViewObject *mview = (PyMemoryViewObject *)arg; + if (mview->view.obj == NULL) { + // Already released, ignore + Py_RETURN_NONE; + } if (mview->view.obj != self) { PyErr_SetString(PyExc_ValueError, "memoryview's buffer is not this object"); @@ -8985,9 +8992,10 @@ bufferwrapper_releasebuf(PyObject *self, Py_buffer *view) Py_TYPE(bw->mv)->tp_as_buffer->bf_releasebuffer(bw->mv, view); // We only need to call bf_releasebuffer if it's a Python function. If it's a C // bf_releasebuf, it will be called when the memoryview is released. - if (Py_TYPE(bw->obj)->tp_as_buffer != NULL - && Py_TYPE(bw->obj)->tp_as_buffer->bf_releasebuffer == slot_bf_releasebuffer) { - Py_TYPE(bw->obj)->tp_as_buffer->bf_releasebuffer(bw->obj, view); + if (((PyMemoryViewObject *)bw->mv)->view.obj != bw->obj + && Py_TYPE(bw->obj)->tp_as_buffer != NULL + && Py_TYPE(bw->obj)->tp_as_buffer->bf_releasebuffer == slot_bf_releasebuffer) { + releasebuffer_call_python(bw->obj, view); } } @@ -9052,8 +9060,51 @@ slot_bf_getbuffer(PyObject *self, Py_buffer *buffer, int flags) return -1; } +static int +releasebuffer_maybe_call_super(PyObject *self, Py_buffer *buffer) +{ + PyTypeObject *self_type = Py_TYPE(self); + PyObject *mro = lookup_tp_mro(self_type); + if (mro == NULL) { + return -1; + } + + assert(PyTuple_Check(mro)); + Py_ssize_t n = PyTuple_GET_SIZE(mro); + Py_ssize_t i; + + /* No need to check the last one: it's gonna be skipped anyway. */ + for (i = 0; i+1 < n; i++) { + if ((PyObject *)(self_type) == PyTuple_GET_ITEM(mro, i)) + break; + } + i++; /* skip self_type */ + if (i >= n) + return -1; + + releasebufferproc base_releasebuffer = NULL; + for (; i < n; i++) { + PyObject *obj = PyTuple_GET_ITEM(mro, i); + if (!PyType_Check(obj)) { + continue; + } + PyTypeObject *base_type = (PyTypeObject *)obj; + if (base_type->tp_as_buffer != NULL + && base_type->tp_as_buffer->bf_releasebuffer != NULL + && base_type->tp_as_buffer->bf_releasebuffer != slot_bf_releasebuffer) { + base_releasebuffer = base_type->tp_as_buffer->bf_releasebuffer; + break; + } + } + + if (base_releasebuffer != NULL) { + base_releasebuffer(self, buffer); + } + return 0; +} + static void -slot_bf_releasebuffer(PyObject *self, Py_buffer *buffer) +releasebuffer_call_python(PyObject *self, Py_buffer *buffer) { PyObject *mv; if (Py_TYPE(buffer->obj) == &_PyBufferWrapper_Type) { @@ -9079,6 +9130,28 @@ slot_bf_releasebuffer(PyObject *self, Py_buffer *buffer) } } +/* + * bf_releasebuffer is very delicate, because we need to ensure that + * C bf_releasebuffer slots are called correctly (or we'll leak memory), + * but we cannot trust any __release_buffer__ implemented in Python to + * do so correctly. Therefore, if a base class has a C bf_releasebuffer + * slot, we call it directly here. That is safe because this function + * only gets called from C callers of the bf_releasebuffer slot. Python + * code that calls __release_buffer__ directly instead goes through + * wrap_releasebuffer(), which doesn't call the bf_releasebuffer slot + * directly but instead simply releases the associated memoryview. + */ +static void +slot_bf_releasebuffer(PyObject *self, Py_buffer *buffer) +{ + if (releasebuffer_maybe_call_super(self, buffer) < 0) { + if (PyErr_Occurred()) { + PyErr_WriteUnraisable(self); + } + } + releasebuffer_call_python(self, buffer); +} + static PyObject * slot_am_await(PyObject *self) { From 88ce24da3d15e86177cb6e710331382a21b28960 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Sat, 6 May 2023 06:39:44 -0700 Subject: [PATCH 03/11] Add a test --- Lib/test/test_buffer.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/Lib/test/test_buffer.py b/Lib/test/test_buffer.py index 44a4fdf7bcc7ca..26ce31ca7ffd85 100644 --- a/Lib/test/test_buffer.py +++ b/Lib/test/test_buffer.py @@ -4604,6 +4604,36 @@ def __release_buffer__(self, view): self.assertEqual(rb_call_count, 0) self.assertEqual(rb_call_count, 1) + def test_inherit_but_return_something_else(self): + class A(bytearray): + def __buffer__(self, flags): + return memoryview(b"hello") + + a = A(b"hello") + with memoryview(a) as mv: + self.assertEqual(mv.tobytes(), b"hello") + + rb_call_count = 0 + rb_raised = False + class B(bytearray): + def __buffer__(self, flags): + return memoryview(b"hello") + def __release_buffer__(self, view): + nonlocal rb_call_count + rb_call_count += 1 + try: + super().__release_buffer__(view) + except ValueError: + nonlocal rb_raised + rb_raised = True + + b = B(b"hello") + with memoryview(b) as mv: + self.assertEqual(mv.tobytes(), b"hello") + self.assertEqual(rb_call_count, 0) + self.assertEqual(rb_call_count, 1) + self.assertIs(rb_raised, True) + if __name__ == "__main__": unittest.main() From 80a4b0b9b3dc7c3bc440272c2079f17e032da64a Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Sat, 6 May 2023 17:45:54 -0700 Subject: [PATCH 04/11] Deal with __release_buffer__ saving references to the buffer --- Include/cpython/memoryobject.h | 1 + Lib/test/test_buffer.py | 36 ++++++++++++++++++++++++++++++++++ Objects/memoryobject.c | 19 ++++++++++++++++++ Objects/typeobject.c | 16 +++++++++++++-- 4 files changed, 70 insertions(+), 2 deletions(-) diff --git a/Include/cpython/memoryobject.h b/Include/cpython/memoryobject.h index deab3cc89f726e..3445b5e2e38f4b 100644 --- a/Include/cpython/memoryobject.h +++ b/Include/cpython/memoryobject.h @@ -24,6 +24,7 @@ typedef struct { #define _Py_MEMORYVIEW_FORTRAN 0x004 /* Fortran contiguous layout */ #define _Py_MEMORYVIEW_SCALAR 0x008 /* scalar: ndim = 0 */ #define _Py_MEMORYVIEW_PIL 0x010 /* PIL-style layout */ +#define _Py_MEMORYVIEW_RESTRICTED 0x020 /* Disallow additional references */ typedef struct { PyObject_VAR_HEAD diff --git a/Lib/test/test_buffer.py b/Lib/test/test_buffer.py index 26ce31ca7ffd85..a8d2c1dc4ec5e3 100644 --- a/Lib/test/test_buffer.py +++ b/Lib/test/test_buffer.py @@ -4634,6 +4634,42 @@ def __release_buffer__(self, view): self.assertEqual(rb_call_count, 1) self.assertIs(rb_raised, True) + def test_override_only_release(self): + class C(bytearray): + def __release_buffer__(self, buffer): + super().__release_buffer__(buffer) + + c = C(b"hello") + with memoryview(c) as mv: + self.assertEqual(mv.tobytes(), b"hello") + + def test_release_saves_reference(self): + smuggled_buffer = None + + class C(bytearray): + def __release_buffer__(s, buffer: memoryview): + with self.assertRaises(ValueError): + memoryview(buffer) + with self.assertRaises(ValueError): + buffer.cast("b") + with self.assertRaises(ValueError): + buffer.toreadonly() + with self.assertRaises(ValueError): + buffer[:1] + with self.assertRaises(ValueError): + buffer.__buffer__(0) + nonlocal smuggled_buffer + smuggled_buffer = buffer + self.assertEqual(buffer.tobytes(), b"hello") + super().__release_buffer__(buffer) + + c = C(b"hello") + with memoryview(c) as mv: + self.assertEqual(mv.tobytes(), b"hello") + c.clear() + with self.assertRaises(ValueError): + smuggled_buffer.tobytes() + if __name__ == "__main__": unittest.main() diff --git a/Objects/memoryobject.c b/Objects/memoryobject.c index 0992a70fade448..b0168044d9f85a 100644 --- a/Objects/memoryobject.c +++ b/Objects/memoryobject.c @@ -193,6 +193,20 @@ PyTypeObject _PyManagedBuffer_Type = { return -1; \ } +#define CHECK_RESTRICTED(mv) \ + if (((PyMemoryViewObject *)(mv))->flags & _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; \ + } + /* See gh-92888. These macros signal that we need to check the memoryview again due to possible read after frees. */ #define CHECK_RELEASED_AGAIN(mv) CHECK_RELEASED(mv) @@ -789,6 +803,7 @@ 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); } else if (PyObject_CheckBuffer(v)) { @@ -1421,6 +1436,7 @@ memoryview_cast_impl(PyMemoryViewObject *self, PyObject *format, Py_ssize_t ndim = 1; CHECK_RELEASED(self); + CHECK_RESTRICTED(self); if (!MV_C_CONTIGUOUS(self->flags)) { PyErr_SetString(PyExc_TypeError, @@ -1476,6 +1492,7 @@ memoryview_toreadonly_impl(PyMemoryViewObject *self) /*[clinic end generated code: output=2c7e056f04c99e62 input=dc06d20f19ba236f]*/ { 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. */ @@ -1498,6 +1515,7 @@ memory_getbuf(PyMemoryViewObject *self, Py_buffer *view, int flags) int baseflags = self->flags; CHECK_RELEASED_INT(self); + CHECK_RESTRICTED_INT(self); /* start with complete information */ *view = *base; @@ -2559,6 +2577,7 @@ memory_subscript(PyMemoryViewObject *self, PyObject *key) return memory_item(self, index); } else if (PySlice_Check(key)) { + CHECK_RESTRICTED(self); PyMemoryViewObject *sliced; sliced = (PyMemoryViewObject *)mbuf_add_view(self->mbuf, view); diff --git a/Objects/typeobject.c b/Objects/typeobject.c index ae27aa5ccb3de7..fb81abc08c01a1 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -9107,27 +9107,39 @@ static void releasebuffer_call_python(PyObject *self, Py_buffer *buffer) { PyObject *mv; + bool is_buffer_wrapper = Py_TYPE(buffer->obj) == &_PyBufferWrapper_Type; if (Py_TYPE(buffer->obj) == &_PyBufferWrapper_Type) { // Make sure we pass the same memoryview to // __release_buffer__() that __buffer__() returned. mv = Py_NewRef(((PyBufferWrapper *)buffer->obj)->mv); } else { + // This means we are not dealing with a memoryview returned + // from a Python __buffer__ function. mv = PyMemoryView_FromBuffer(buffer); if (mv == NULL) { PyErr_WriteUnraisable(self); return; } + // Set the memoryview to restricted mode, which forbids + // users from saving any reference to the underlying buffer + // (e.g., by doing .cast()). This is necessary to ensure + // no Python code retains a reference to the to-be-released + // buffer. + ((PyMemoryViewObject *)mv)->flags |= _Py_MEMORYVIEW_RESTRICTED; } PyObject *stack[2] = {self, mv}; PyObject *ret = vectorcall_method(&_Py_ID(__release_buffer__), stack, 2); - Py_DECREF(mv); if (ret == NULL) { PyErr_WriteUnraisable(self); } else { Py_DECREF(ret); } + if (!is_buffer_wrapper) { + PyObject_CallMethodNoArgs(mv, &_Py_ID(release)); + } + Py_DECREF(mv); } /* @@ -9144,12 +9156,12 @@ releasebuffer_call_python(PyObject *self, Py_buffer *buffer) static void slot_bf_releasebuffer(PyObject *self, Py_buffer *buffer) { + releasebuffer_call_python(self, buffer); if (releasebuffer_maybe_call_super(self, buffer) < 0) { if (PyErr_Occurred()) { PyErr_WriteUnraisable(self); } } - releasebuffer_call_python(self, buffer); } static PyObject * From 6e316646293f6e1344470ea50089ff45c48a3d43 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Sat, 6 May 2023 18:01:53 -0700 Subject: [PATCH 05/11] Add another test --- Lib/test/test_buffer.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/Lib/test/test_buffer.py b/Lib/test/test_buffer.py index a8d2c1dc4ec5e3..dc4ce54a53f8fc 100644 --- a/Lib/test/test_buffer.py +++ b/Lib/test/test_buffer.py @@ -4670,6 +4670,26 @@ def __release_buffer__(s, buffer: memoryview): with self.assertRaises(ValueError): smuggled_buffer.tobytes() + def test_release_saves_reference_no_subclassing(self): + ba = bytearray(b"hello") + + class C: + def __buffer__(self, flags): + return memoryview(ba) + + def __release_buffer__(self, buffer): + self.buffer = buffer + + c = C() + with memoryview(c) as mv: + self.assertEqual(mv.tobytes(), b"hello") + self.assertEqual(c.buffer.tobytes(), b"hello") + + with self.assertRaises(BufferError): + ba.clear() + c.buffer.release() + ba.clear() + if __name__ == "__main__": unittest.main() From b73e29c6cb78d16c4a5e76ee3368eb4d62dd050c Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Sat, 6 May 2023 18:29:02 -0700 Subject: [PATCH 06/11] More paranoid bufferwrapper_releasebuf --- Objects/typeobject.c | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index fb81abc08c01a1..a4a0cdf2ed82af 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -8988,15 +8988,31 @@ bufferwrapper_releasebuf(PyObject *self, Py_buffer *view) { PyBufferWrapper *bw = (PyBufferWrapper *)self; - assert(PyMemoryView_Check(bw->mv)); - Py_TYPE(bw->mv)->tp_as_buffer->bf_releasebuffer(bw->mv, view); + if (bw->mv == NULL || bw->obj == NULL) { + // Already released + return; + } + + PyObject *mv = Py_NewRef(bw->mv); + PyObject *obj = Py_NewRef(bw->obj); + + // Clear these fields first, in case we somehow get called + // recursively. + Py_CLEAR(bw->mv); + Py_CLEAR(bw->obj); + + assert(PyMemoryView_Check(mv)); + Py_TYPE(mv)->tp_as_buffer->bf_releasebuffer(mv, view); // We only need to call bf_releasebuffer if it's a Python function. If it's a C // bf_releasebuf, it will be called when the memoryview is released. - if (((PyMemoryViewObject *)bw->mv)->view.obj != bw->obj - && Py_TYPE(bw->obj)->tp_as_buffer != NULL - && Py_TYPE(bw->obj)->tp_as_buffer->bf_releasebuffer == slot_bf_releasebuffer) { - releasebuffer_call_python(bw->obj, view); + if (((PyMemoryViewObject *)mv)->view.obj != obj + && Py_TYPE(obj)->tp_as_buffer != NULL + && Py_TYPE(obj)->tp_as_buffer->bf_releasebuffer == slot_bf_releasebuffer) { + releasebuffer_call_python(obj, view); } + + Py_DECREF(mv); + Py_DECREF(obj); } static PyBufferProcs bufferwrapper_as_buffer = { @@ -9111,7 +9127,11 @@ releasebuffer_call_python(PyObject *self, Py_buffer *buffer) if (Py_TYPE(buffer->obj) == &_PyBufferWrapper_Type) { // Make sure we pass the same memoryview to // __release_buffer__() that __buffer__() returned. - mv = Py_NewRef(((PyBufferWrapper *)buffer->obj)->mv); + PyBufferWrapper *bw = (PyBufferWrapper *)buffer->obj; + if (bw->mv == NULL) { + return; + } + mv = Py_NewRef(bw->mv); } else { // This means we are not dealing with a memoryview returned From 15ad02d889fc7781d5efc309eb9287f2e035a806 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Sat, 6 May 2023 20:01:57 -0700 Subject: [PATCH 07/11] that did not work --- Objects/typeobject.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index a4a0cdf2ed82af..50c93c55a6059f 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -8993,13 +8993,8 @@ bufferwrapper_releasebuf(PyObject *self, Py_buffer *view) return; } - PyObject *mv = Py_NewRef(bw->mv); - PyObject *obj = Py_NewRef(bw->obj); - - // Clear these fields first, in case we somehow get called - // recursively. - Py_CLEAR(bw->mv); - Py_CLEAR(bw->obj); + PyObject *mv = bw->mv; + PyObject *obj = bw->obj; assert(PyMemoryView_Check(mv)); Py_TYPE(mv)->tp_as_buffer->bf_releasebuffer(mv, view); @@ -9011,8 +9006,8 @@ bufferwrapper_releasebuf(PyObject *self, Py_buffer *view) releasebuffer_call_python(obj, view); } - Py_DECREF(mv); - Py_DECREF(obj); + Py_CLEAR(bw->mv); + Py_CLEAR(bw->obj); } static PyBufferProcs bufferwrapper_as_buffer = { From 9ded8731ff27c227ba73ed6edbb7508d49e9048a Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Sun, 7 May 2023 07:14:33 -0700 Subject: [PATCH 08/11] Update Objects/typeobject.c Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> --- Objects/typeobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 50c93c55a6059f..2e1c6b1500cdcb 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -9085,7 +9085,7 @@ releasebuffer_maybe_call_super(PyObject *self, Py_buffer *buffer) Py_ssize_t i; /* No need to check the last one: it's gonna be skipped anyway. */ - for (i = 0; i+1 < n; i++) { + for (i = 0; i < n -1; i++) { if ((PyObject *)(self_type) == PyTuple_GET_ITEM(mro, i)) break; } From 3b50138960f77d6a52a1623fe5bfeee934191bfc Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Sun, 7 May 2023 07:14:45 -0700 Subject: [PATCH 09/11] Update Objects/typeobject.c Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> --- Objects/typeobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 2e1c6b1500cdcb..98fac276a873e1 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -9119,7 +9119,7 @@ releasebuffer_call_python(PyObject *self, Py_buffer *buffer) { PyObject *mv; bool is_buffer_wrapper = Py_TYPE(buffer->obj) == &_PyBufferWrapper_Type; - if (Py_TYPE(buffer->obj) == &_PyBufferWrapper_Type) { + if (is_buffer_wrapper) { // Make sure we pass the same memoryview to // __release_buffer__() that __buffer__() returned. PyBufferWrapper *bw = (PyBufferWrapper *)buffer->obj; From 9ee8eb5d59425416bc68cc7dd3d9a4291cbe3cb1 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Sun, 7 May 2023 07:25:14 -0700 Subject: [PATCH 10/11] Better message for new flag --- Include/cpython/memoryobject.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Include/cpython/memoryobject.h b/Include/cpython/memoryobject.h index 3445b5e2e38f4b..3837fa8c6ab5aa 100644 --- a/Include/cpython/memoryobject.h +++ b/Include/cpython/memoryobject.h @@ -24,7 +24,7 @@ typedef struct { #define _Py_MEMORYVIEW_FORTRAN 0x004 /* Fortran contiguous layout */ #define _Py_MEMORYVIEW_SCALAR 0x008 /* scalar: ndim = 0 */ #define _Py_MEMORYVIEW_PIL 0x010 /* PIL-style layout */ -#define _Py_MEMORYVIEW_RESTRICTED 0x020 /* Disallow additional references */ +#define _Py_MEMORYVIEW_RESTRICTED 0x020 /* Disallow new references to the memoryview's buffer */ typedef struct { PyObject_VAR_HEAD From 5536853ed0f11c7e5bb38163af53e16e51432dc9 Mon Sep 17 00:00:00 2001 From: Jelle Zijlstra Date: Sun, 7 May 2023 07:25:18 -0700 Subject: [PATCH 11/11] More tests --- Lib/test/test_buffer.py | 59 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/Lib/test/test_buffer.py b/Lib/test/test_buffer.py index dc4ce54a53f8fc..2c65ae8114818f 100644 --- a/Lib/test/test_buffer.py +++ b/Lib/test/test_buffer.py @@ -4690,6 +4690,65 @@ def __release_buffer__(self, buffer): c.buffer.release() ba.clear() + def test_multiple_inheritance_buffer_last(self): + class A: + def __buffer__(self, flags): + return memoryview(b"hello A") + + class B(A, bytearray): + def __buffer__(self, flags): + return super().__buffer__(flags) + + b = B(b"hello") + with memoryview(b) as mv: + self.assertEqual(mv.tobytes(), b"hello A") + + class Releaser: + def __release_buffer__(self, buffer): + self.buffer = buffer + + class C(Releaser, bytearray): + def __buffer__(self, flags): + return super().__buffer__(flags) + + c = C(b"hello C") + with memoryview(c) as mv: + self.assertEqual(mv.tobytes(), b"hello C") + c.clear() + with self.assertRaises(ValueError): + c.buffer.tobytes() + + def test_multiple_inheritance_buffer_last(self): + class A: + def __buffer__(self, flags): + raise RuntimeError("should not be called") + + def __release_buffer__(self, buffer): + raise RuntimeError("should not be called") + + class B(bytearray, A): + def __buffer__(self, flags): + return super().__buffer__(flags) + + b = B(b"hello") + with memoryview(b) as mv: + self.assertEqual(mv.tobytes(), b"hello") + + class Releaser: + buffer = None + def __release_buffer__(self, buffer): + self.buffer = buffer + + class C(bytearray, Releaser): + def __buffer__(self, flags): + return super().__buffer__(flags) + + c = C(b"hello") + with memoryview(c) as mv: + self.assertEqual(mv.tobytes(), b"hello") + c.clear() + self.assertIs(c.buffer, None) + if __name__ == "__main__": unittest.main()