Update implementation of PEP 3118 getbuffer special methoddalcinl/fix-getbuffer

* Rework check for `view==NULL` required for legacy Python 3 releases.
* Handle redefined `struct Py_buffer` in user code (e.g. mpi4py).

3 files changed, 141 insertions, 38 deletions

diff --git a/Cython/Compiler/Nodes.py b/Cython/Compiler/Nodes.py
index e493d2b01..e0fb4b5bc 100644
--- a/Cython/Compiler/Nodes.py
+++ b/Cython/Compiler/Nodes.py
@@ -1854,6 +1854,10 @@ class FuncDefNode(StatNode, BlockNode):
                 code_object = self.code_object.calculate_result_code(code) if self.code_object else None
                 code.put_trace_frame_init(code_object)
 
+        # ----- Special check for getbuffer
+        if is_getbuffer_slot:
+            self.getbuffer_check(code)
+
         # ----- set up refnanny
         if use_refnanny:
             tempvardecl_code.put_declare_refcount_context()
@@ -2201,31 +2205,59 @@ class FuncDefNode(StatNode, BlockNode):
     #
     # Special code for the __getbuffer__ function
     #
-    def getbuffer_init(self, code):
-        info = self.local_scope.arg_entries[1].cname
-        # Python 3.0 betas have a bug in memoryview which makes it call
-        # getbuffer with a NULL parameter. For now we work around this;
-        # the following block should be removed when this bug is fixed.
-        code.putln("if (%s != NULL) {" % info)
-        code.putln("%s->obj = Py_None; __Pyx_INCREF(Py_None);" % info)
-        code.put_giveref("%s->obj" % info) # Do not refnanny object within structs
+    def _get_py_buffer_info(self):
+        py_buffer = self.local_scope.arg_entries[1]
+        try:
+            # Check builtin definition of struct Py_buffer
+            obj_type = py_buffer.type.base_type.scope.entries['obj'].type
+        except (AttributeError, KeyError):
+            # User code redeclared struct Py_buffer
+            obj_type = None
+        return py_buffer, obj_type
+
+    # Old Python 3 used to support write-locks on buffer-like objects by
+    # calling PyObject_GetBuffer() with a view==NULL parameter. This obscure
+    # feature is obsolete, it was almost never used (only one instance in
+    # `Modules/posixmodule.c` in Python 3.1) and it is now officially removed
+    # (see bpo-14203). We add an extra check here to prevent legacy code from
+    # from trying to use the feature and prevent segmentation faults.
+    def getbuffer_check(self, code):
+        py_buffer, _ = self._get_py_buffer_info()
+        view = py_buffer.cname
+        code.putln("if (%s == NULL) {" % view)
+        code.putln("PyErr_SetString(PyExc_BufferError, "
+                   "\"PyObject_GetBuffer: view==NULL argument is obsolete\");")
+        code.putln("return -1;")
         code.putln("}")
 
+    def getbuffer_init(self, code):
+        py_buffer, obj_type = self._get_py_buffer_info()
+        view = py_buffer.cname
+        if obj_type and obj_type.is_pyobject:
+            code.put_init_to_py_none("%s->obj" % view, obj_type)
+            code.put_giveref("%s->obj" % view) # Do not refnanny object within structs
+        else:
+            code.putln("%s->obj = NULL;" % view)
+
     def getbuffer_error_cleanup(self, code):
-        info = self.local_scope.arg_entries[1].cname
-        code.putln("if (%s != NULL && %s->obj != NULL) {"
-                   % (info, info))
-        code.put_gotref("%s->obj" % info)
-        code.putln("__Pyx_DECREF(%s->obj); %s->obj = NULL;"
-                   % (info, info))
-        code.putln("}")
+        py_buffer, obj_type = self._get_py_buffer_info()
+        view = py_buffer.cname
+        if obj_type and obj_type.is_pyobject:
+            code.putln("if (%s->obj != NULL) {" % view)
+            code.put_gotref("%s->obj" % view)
+            code.put_decref_clear("%s->obj" % view, obj_type)
+            code.putln("}")
+        else:
+            code.putln("Py_CLEAR(%s->obj);" % view)
 
     def getbuffer_normal_cleanup(self, code):
-        info = self.local_scope.arg_entries[1].cname
-        code.putln("if (%s != NULL && %s->obj == Py_None) {" % (info, info))
-        code.put_gotref("Py_None")
-        code.putln("__Pyx_DECREF(Py_None); %s->obj = NULL;" % info)
-        code.putln("}")
+        py_buffer, obj_type = self._get_py_buffer_info()
+        view = py_buffer.cname
+        if obj_type and obj_type.is_pyobject:
+            code.putln("if (%s->obj == Py_None) {" % view)
+            code.put_gotref("%s->obj" % view)
+            code.put_decref_clear("%s->obj" % view, obj_type)
+            code.putln("}")
 
     def get_preprocessor_guard(self):
         if not self.entry.is_special:
diff --git a/Cython/Includes/numpy/__init__.pxd b/Cython/Includes/numpy/__init__.pxd
index 3a6c67a96..bf98e0557 100644
--- a/Cython/Includes/numpy/__init__.pxd
+++ b/Cython/Includes/numpy/__init__.pxd
@@ -217,19 +217,12 @@ cdef extern from "numpy/arrayobject.h":
             # In particular strided access is always provided regardless
             # of flags
 
-            if info == NULL: return
-
-            cdef int copy_shape, i, ndim
+            cdef int i, ndim
             cdef int endian_detector = 1
             cdef bint little_endian = ((<char*>&endian_detector)[0] != 0)
 
             ndim = PyArray_NDIM(self)
 
-            if sizeof(npy_intp) != sizeof(Py_ssize_t):
-                copy_shape = 1
-            else:
-                copy_shape = 0
-
             if ((flags & pybuf.PyBUF_C_CONTIGUOUS == pybuf.PyBUF_C_CONTIGUOUS)
                 and not PyArray_CHKFLAGS(self, NPY_C_CONTIGUOUS)):
                 raise ValueError(u"ndarray is not C contiguous")
@@ -240,7 +233,7 @@ cdef extern from "numpy/arrayobject.h":
 
             info.buf = PyArray_DATA(self)
             info.ndim = ndim
-            if copy_shape:
+            if sizeof(npy_intp) != sizeof(Py_ssize_t):
                 # Allocate new buffer for strides and shape info.
                 # This is allocated as one block, strides first.
                 info.strides = <Py_ssize_t*>PyObject_Malloc(sizeof(Py_ssize_t) * 2 * <size_t>ndim)
@@ -260,16 +253,9 @@ cdef extern from "numpy/arrayobject.h":
             cdef dtype descr = self.descr
             cdef int offset
 
-            cdef bint hasfields = PyDataType_HASFIELDS(descr)
-
-            if not hasfields and not copy_shape:
-                # do not call releasebuffer
-                info.obj = None
-            else:
-                # need to call releasebuffer
-                info.obj = self
+            info.obj = self
 
-            if not hasfields:
+            if not PyDataType_HASFIELDS(descr):
                 t = descr.type_num
                 if ((descr.byteorder == c'>' and little_endian) or
                     (descr.byteorder == c'<' and not little_endian)):
diff --git a/tests/buffers/userbuffer.pyx b/tests/buffers/userbuffer.pyx
new file mode 100644
index 000000000..b9c871970
--- /dev/null
+++ b/tests/buffers/userbuffer.pyx
@@ -0,0 +1,85 @@
+import sys
+
+__doc__ = u""
+
+if sys.version_info[:2] == (2, 6):
+    __doc__ += u"""
+>>> memoryview = _memoryview
+"""
+
+__doc__ += u"""
+>>> b1 = UserBuffer1()
+>>> m1 = memoryview(b1)
+>>> m1.tolist()
+[0, 1, 2, 3, 4]
+>>> del m1, b1
+"""
+
+__doc__ += u"""
+>>> b2 = UserBuffer2()
+>>> m2 = memoryview(b2)
+UserBuffer2: getbuffer
+>>> m2.tolist()
+[5, 6, 7, 8, 9]
+>>> del m2, b2
+UserBuffer2: release
+"""
+
+cdef extern from *:
+    ctypedef struct Py_buffer # redeclared
+    enum: PyBUF_SIMPLE
+    int PyBuffer_FillInfo(Py_buffer *, object, void *, Py_ssize_t, bint, int) except -1
+    int  PyObject_GetBuffer(object, Py_buffer *, int) except -1
+    void PyBuffer_Release(Py_buffer *)
+
+cdef char global_buf[5]
+global_buf[0:5] = [0, 1, 2, 3, 4]
+
+cdef class UserBuffer1:
+
+    def __getbuffer__(self, Py_buffer* view, int flags):
+        PyBuffer_FillInfo(view, None, global_buf, 5, 1, flags)
+
+cdef class UserBuffer2:
+    cdef char buf[5]
+
+    def __cinit__(self):
+        self.buf[0:5] = [5, 6, 7, 8, 9]
+
+    def __getbuffer__(self, Py_buffer* view, int flags):
+        print('UserBuffer2: getbuffer')
+        PyBuffer_FillInfo(view, self, self.buf, 5, 0, flags)
+
+    def __releasebuffer__(self, Py_buffer* view):
+        print('UserBuffer2: release')
+
+
+cdef extern from *:
+    ctypedef struct PyBuffer"Py_buffer":
+        void *buf
+        Py_ssize_t len
+        bint readonly
+
+cdef class _memoryview:
+
+    """
+    Memory
+    """
+
+    cdef PyBuffer view
+
+    def __cinit__(self, obj):
+        cdef Py_buffer *view = <Py_buffer*>&self.view
+        PyObject_GetBuffer(obj, view, PyBUF_SIMPLE)
+
+    def __dealloc__(self):
+        cdef Py_buffer *view = <Py_buffer*>&self.view
+        PyBuffer_Release(view )
+        
+    def __getbuffer__(self, Py_buffer *view, int flags):
+        PyBuffer_FillInfo(view, self,
+                          self.view.buf, self.view.len,
+                          self.view.readonly, flags)
+    def tolist(self):
+        cdef char *b = <char *> self.view.buf
+        return [b[i] for i in range(self.view.len)]