From 1bb651540e0743c4e58d875d1de240597862fa34 Mon Sep 17 00:00:00 2001 From: Martin Panter Date: Fri, 13 Nov 2015 21:43:39 +0000 Subject: [PATCH] Issue #25498: Fix GC crash due to ctypes objects wrapping a memoryview This was a regression caused by revision 1da9630e9b7f. Based on patch by Eryksun. --- Lib/ctypes/test/test_frombuffer.py | 31 +++++++++++++++-- Misc/ACKS | 1 + Misc/NEWS | 4 +++ Modules/_ctypes/_ctypes.c | 56 ++++++++++++++++++++---------- 4 files changed, 72 insertions(+), 20 deletions(-) diff --git a/Lib/ctypes/test/test_frombuffer.py b/Lib/ctypes/test/test_frombuffer.py index 6aa2d1c591..86954fe61e 100644 --- a/Lib/ctypes/test/test_frombuffer.py +++ b/Lib/ctypes/test/test_frombuffer.py @@ -38,11 +38,32 @@ class Test(unittest.TestCase): del a; gc.collect(); gc.collect(); gc.collect() self.assertEqual(x[:], expected) - with self.assertRaises(TypeError): + with self.assertRaisesRegex(TypeError, "not writable"): (c_char * 16).from_buffer(b"a" * 16) - with self.assertRaises(TypeError): + with self.assertRaisesRegex(TypeError, "not writable"): + (c_char * 16).from_buffer(memoryview(b"a" * 16)) + with self.assertRaisesRegex(TypeError, "not C contiguous"): + (c_char * 16).from_buffer(memoryview(bytearray(b"a" * 16))[::-1]) + msg = "does not have the buffer interface" + with self.assertRaisesRegex(TypeError, msg): (c_char * 16).from_buffer("a" * 16) + def test_fortran_contiguous(self): + try: + import _testbuffer + except ImportError as err: + self.skipTest(str(err)) + flags = _testbuffer.ND_WRITABLE | _testbuffer.ND_FORTRAN + array = _testbuffer.ndarray( + [97] * 16, format="B", shape=[4, 4], flags=flags) + with self.assertRaisesRegex(TypeError, "not C contiguous"): + (c_char * 16).from_buffer(array) + array = memoryview(array) + self.assertTrue(array.f_contiguous) + self.assertFalse(array.c_contiguous) + with self.assertRaisesRegex(TypeError, "not C contiguous"): + (c_char * 16).from_buffer(array) + def test_from_buffer_with_offset(self): a = array.array("i", range(16)) x = (c_int * 15).from_buffer(a, sizeof(c_int)) @@ -55,6 +76,12 @@ class Test(unittest.TestCase): with self.assertRaises(ValueError): (c_int * 1).from_buffer(a, 16 * sizeof(c_int)) + def test_from_buffer_memoryview(self): + a = [c_char.from_buffer(memoryview(bytearray(b'a')))] + a.append(a) + del a + gc.collect() # Should not crash + def test_from_buffer_copy(self): a = array.array("i", range(16)) x = (c_int * 16).from_buffer_copy(a) diff --git a/Misc/ACKS b/Misc/ACKS index abf73045c0..c9d69e2f36 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -391,6 +391,7 @@ Gökcen Eraslan Stoffel Erasmus Jürgen A. Erhard Michael Ernst +Eryksun Ben Escoto Andy Eskilsson André Espaze diff --git a/Misc/NEWS b/Misc/NEWS index 4e5ea1901b..aacd51ea07 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -103,6 +103,10 @@ Core and Builtins Library ------- +- Issue #25498: Fix a crash when garbage-collecting ctypes objects created + by wrapping a memoryview. This was a regression made in 3.4.3. Based + on patch by Eryksun. + - Issue #18010: Fix the pydoc web server's module search function to handle exceptions from importing packages. diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index 2e5939c257..2b27ed36db 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -463,45 +463,65 @@ KeepRef(CDataObject *target, Py_ssize_t index, PyObject *keep); static PyObject * CDataType_from_buffer(PyObject *type, PyObject *args) { - Py_buffer buffer; + PyObject *obj; + PyObject *mv; + PyObject *result; + Py_buffer *buffer; Py_ssize_t offset = 0; - PyObject *result, *mv; + StgDictObject *dict = PyType_stgdict(type); assert (dict); - if (!PyArg_ParseTuple(args, "w*|n:from_buffer", &buffer, &offset)) + if (!PyArg_ParseTuple(args, "O|n:from_buffer", &obj, &offset)) return NULL; + mv = PyMemoryView_FromObject(obj); + if (mv == NULL) + return NULL; + + buffer = PyMemoryView_GET_BUFFER(mv); + + if (buffer->readonly) { + PyErr_SetString(PyExc_TypeError, + "underlying buffer is not writable"); + Py_DECREF(mv); + return NULL; + } + + if (!PyBuffer_IsContiguous(buffer, 'C')) { + PyErr_SetString(PyExc_TypeError, + "underlying buffer is not C contiguous"); + Py_DECREF(mv); + return NULL; + } + if (offset < 0) { PyErr_SetString(PyExc_ValueError, "offset cannot be negative"); - PyBuffer_Release(&buffer); + Py_DECREF(mv); return NULL; } - if (dict->size > buffer.len - offset) { + + if (dict->size > buffer->len - offset) { PyErr_Format(PyExc_ValueError, - "Buffer size too small (%zd instead of at least %zd bytes)", - buffer.len, dict->size + offset); - PyBuffer_Release(&buffer); + "Buffer size too small " + "(%zd instead of at least %zd bytes)", + buffer->len, dict->size + offset); + Py_DECREF(mv); return NULL; } - result = PyCData_AtAddress(type, (char *)buffer.buf + offset); + result = PyCData_AtAddress(type, (char *)buffer->buf + offset); if (result == NULL) { - PyBuffer_Release(&buffer); + Py_DECREF(mv); return NULL; } - mv = PyMemoryView_FromBuffer(&buffer); - if (mv == NULL) { - PyBuffer_Release(&buffer); + if (-1 == KeepRef((CDataObject *)result, -1, mv)) { + Py_DECREF(result); return NULL; } - /* Hack the memoryview so that it will release the buffer. */ - ((PyMemoryViewObject *)mv)->mbuf->master.obj = buffer.obj; - ((PyMemoryViewObject *)mv)->view.obj = buffer.obj; - if (-1 == KeepRef((CDataObject *)result, -1, mv)) - result = NULL; + return result; } -- 2.40.0