Clean up memoryview reference counting on error (GH-4476)

Fixes https://github.com/cython/cython/issues/4296

If there was an error in preparing the function arguments after a
memoryview had already been created, then the memoryview was not
cleaned up correctly.

(This leaves it in the slightly odd position where memoryviews
are cleaned up in the wrapper function on failure, but in the
main function on success. I kind of think it'd be better to clean
them up in the wrapper function in both cases, but I'm reluctant
to mess with a system that largely works.)

2 files changed, 38 insertions, 0 deletions

diff --git a/Cython/Compiler/Nodes.py b/Cython/Compiler/Nodes.py
index 977cc12a7..12fdc65b7 100644
--- a/Cython/Compiler/Nodes.py
+++ b/Cython/Compiler/Nodes.py
@@ -3740,6 +3740,11 @@ class DefNodeWrapper(FuncDefNode):
                         code.put_var_xdecref_clear(self.starstar_arg.entry)
                     else:
                         code.put_var_decref_clear(self.starstar_arg.entry)
+            for arg in self.args:
+                if not arg.type.is_pyobject and arg.type.needs_refcounting:
+                    # at the moment this just catches memoryviewslices, but in future
+                    # other non-PyObject reference counted types might need cleanup
+                    code.put_var_xdecref(arg.entry)
             code.put_add_traceback(self.target.entry.qualified_name)
             code.put_finish_refcount_context()
             code.putln("return %s;" % self.error_value())
diff --git a/tests/memoryview/memoryview.pyx b/tests/memoryview/memoryview.pyx
index 273a1fa68..bb8b73780 100644
--- a/tests/memoryview/memoryview.pyx
+++ b/tests/memoryview/memoryview.pyx
@@ -1172,3 +1172,36 @@ def test_assign_from_byteslike(byteslike):
         return (<unsigned char*>buf)[:5]
     finally:
         free(buf)
+
+def multiple_memoryview_def(double[:] a, double[:] b):
+    return a[0] + b[0]
+
+cpdef multiple_memoryview_cpdef(double[:] a, double[:] b):
+    return a[0] + b[0]
+
+cdef multiple_memoryview_cdef(double[:] a, double[:] b):
+    return a[0] + b[0]
+
+multiple_memoryview_cdef_wrapper = multiple_memoryview_cdef
+
+def test_conversion_failures():
+    """
+    What we're concerned with here is that we don't lose references if one
+    of several memoryview arguments fails to convert.
+
+    >>> test_conversion_failures()
+    """
+    imb = IntMockBuffer("", range(1), shape=(1,))
+    dmb = DoubleMockBuffer("", range(1), shape=(1,))
+    for first, second in [(imb, dmb), (dmb, imb)]:
+        for func in [multiple_memoryview_def, multiple_memoryview_cpdef, multiple_memoryview_cdef_wrapper]:
+            # note - using python call of "multiple_memoryview_cpdef" deliberately
+            imb_before = get_refcount(imb)
+            dmb_before = get_refcount(dmb)
+            try:
+                func(first, second)
+            except:
+                assert get_refcount(imb) == imb_before, "before %s after %s" % (imb_before, get_refcount(imb))
+                assert get_refcount(dmb) == dmb_before, "before %s after %s" % (dmb_before, get_refcount(dmb))
+            else:
+                assert False, "Conversion should fail!"