[Python-checkins] cpython (3.4): Issue #25498: Fix GC crash due to ctypes objects wrapping a memoryview

Fri Nov 13 17:20:11 EST 2015

https://hg.python.org/cpython/rev/9b23f7a94ba9
changeset: 99110:9b23f7a94ba9
branch: 3.4
parent: 99107:4569663e6e17
user: Martin Panter <vadmium+py at gmail.com>
date: Fri Nov 13 21:43:39 2015 +0000
summary:
 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.
files:
 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
--- a/Lib/ctypes/test/test_frombuffer.py
+++ b/Lib/ctypes/test/test_frombuffer.py
@@ -38,11 +38,32 @@
 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 @@
 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
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -391,6 +391,7 @@
 Stoffel Erasmus
 Jürgen A. Erhard
 Michael Ernst
+Eryksun
 Ben Escoto
 Andy Eskilsson
 André Espaze
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -103,6 +103,10 @@
 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
--- a/Modules/_ctypes/_ctypes.c
+++ b/Modules/_ctypes/_ctypes.c
@@ -463,45 +463,65 @@
 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;
 }
 
-- 
Repository URL: https://hg.python.org/cpython
