[Cython] memoryview refcount error

[Syam Gadde]

I've done some more digging here. It turns out the test case for this 
bug is simpler than I thought:

import numpy
cimport numpy

def testfunc():
    a = numpy.arange(12).reshape([3,4])
    cdef numpy.int_t[:,:] a_view = a
    cdef numpy.int_t[:,:] b = a_view[:,:].T

testfunc()


The core issue is that a transpose of a slice of a memoryview results in 
bad reference counts on the memview (comments interspersed below to 
indicate refcount and who has references to the base memoryview):

   /* "testtranspose.pyx":6
  * def testfunc():
  *    a = numpy.arange(12).reshape([3,4])
  *    cdef numpy.int_t[:,:] a_view = a             # <<<<<<<<<<<<<<
  *    cdef numpy.int_t[:,:] b = a_view[:,:].T
  *
  */
   __pyx_t_4 = 
__Pyx_PyObject_to_MemoryviewSlice_dsds_nn___pyx_t_5numpy_int_t(__pyx_v_a);
   if (unlikely(!__pyx_t_4.memview)) {__pyx_filename = __pyx_f[0]; 
__pyx_lineno = 6; __pyx_clineno = __LINE__; goto __pyx_L1_error;}
   __pyx_v_a_view = __pyx_t_4;
   __pyx_t_4.memview = NULL;
   __pyx_t_4.data = NULL;
// >>> [SG] REFCOUNT=1 (pointed to by __pyx_v_a_view)


   /* "testtranspose.pyx":7
  *    a = numpy.arange(12).reshape([3,4])
  *    cdef numpy.int_t[:,:] a_view = a
  *    cdef numpy.int_t[:,:] b = a_view[:,:].T # <<<<<<<<<<<<<<
  *
  * testfunc()
  */
   __pyx_t_6 = -1;
   __pyx_t_5.data = __pyx_v_a_view.data;
   __pyx_t_5.memview = __pyx_v_a_view.memview;
   __PYX_INC_MEMVIEW(&__pyx_t_5, 0);
// >>> [SG] REFCOUNT=2 (pointed to by __pyx_v_a_view, __pyx_t_5)
   __pyx_t_5.shape[0] = __pyx_v_a_view.shape[0];
__pyx_t_5.strides[0] = __pyx_v_a_view.strides[0];
     __pyx_t_5.suboffsets[0] = -1;

__pyx_t_5.shape[1] = __pyx_v_a_view.shape[1];
__pyx_t_5.strides[1] = __pyx_v_a_view.strides[1];
     __pyx_t_5.suboffsets[1] = -1;

__pyx_t_7 = __pyx_t_5;
// >>> [SG] REFCOUNT=2 (pointed to by __pyx_v_a_view, __pyx_t_5, __pyx_t_7)
   if (unlikely(__pyx_memslice_transpose(&__pyx_t_7) == 0)) 
{__pyx_filename = __pyx_f[0]; __pyx_lineno = 7; __pyx_clineno = 
__LINE__; goto __pyx_L1_error;}
   __PYX_XDEC_MEMVIEW(&__pyx_t_5, 1);
// >>> [SG] REFCOUNT=1 (pointed to by __pyx_v_a_view, __pyx_t_5, __pyx_t_7)
   __pyx_v_b = __pyx_t_7;
// >>> [SG] REFCOUNT=1 (pointed to by __pyx_v_a_view, __pyx_t_5, 
__pyx_t_7, __pyx_v_b)
   __pyx_t_7.memview = NULL;
   __pyx_t_7.data = NULL;
// >>> [SG] REFCOUNT=1 (pointed to by __pyx_v_a_view, __pyx_t_5, __pyx_v_b)

Note that now __pyx_v_a_view, __pyx_t_b and __pyx_t_5 retain references 
to the same memview, but the memview's reference count is only 1.  At 
the end of the function, the memviews of both __pyx_v_a_view and 
__pyx_v_b will both get decremented, causing a segfault.  If any error 
happens before that, then it will also attempt a decrement of __pyx_t_5 
(which is really, really annoying because the resulting segfault loses 
the actual error).  I think __pyx_t_7 needs an increment before the call 
to memslice_transpose, and that __pyx_t_5.memview needs to be set to 
NULL after its count is decremented.  The following patch fixes the 
issue, but as I'm not very familiar with Cython internals, it may be 
doing too much, or doing it in the wrong place.

-syam


diff --git a/Cython/Compiler/ExprNodes.py b/Cython/Compiler/ExprNodes.py
index 315d6bb..631db8b 100644
--- a/Cython/Compiler/ExprNodes.py
+++ b/Cython/Compiler/ExprNodes.py
@@ -614,6 +614,8 @@ class ExprNode(Node):
                  elif self.type.is_memoryviewslice:
                      code.put_xdecref_memoryviewslice(
                              self.result(), have_gil=not 
self.in_nogil_context)
+                    code.putln("%s.memview = NULL;" % self.result())
+                    code.putln("%s.data = NULL;" % self.result())
          else:
              # Already done if self.is_temp
              self.generate_subexpr_disposal_code(code)
@@ -5561,9 +5563,10 @@ class AttributeNode(ExprNode):
                          return

                  code.putln("%s = %s;" % (self.result(), 
self.obj.result()))
-                if self.obj.is_name or (self.obj.is_attribute and
- self.obj.is_memslice_transpose):
- code.put_incref_memoryviewslice(self.result(), have_gil=True)
+                #if self.obj.is_name or (self.obj.is_attribute and
+                # self.obj.is_memslice_transpose):
+                # code.put_incref_memoryviewslice(self.result(), 
have_gil=True)
+                code.put_incref_memoryviewslice(self.result(), 
have_gil=True)

                  T = "__pyx_memslice_transpose(&%s) == 0"
                  code.putln(code.error_goto_if(T % self.result(), 
self.pos))
@@ -5586,10 +5589,14 @@ class AttributeNode(ExprNode):
      def generate_disposal_code(self, code):
          if self.is_temp and self.type.is_memoryviewslice and 
self.is_memslice_transpose:
              # mirror condition for putting the memview incref here:
-            if self.obj.is_name or (self.obj.is_attribute and
- self.obj.is_memslice_transpose):
-                code.put_xdecref_memoryviewslice(
+            #if self.obj.is_name or (self.obj.is_attribute and
+            # self.obj.is_memslice_transpose):
+            #    code.put_xdecref_memoryviewslice(
+            #            self.result(), have_gil=True)
+            code.put_xdecref_memoryviewslice(
                          self.result(), have_gil=True)
+            code.putln("%s.memview = NULL;" % self.result())
+            code.putln("%s.data = NULL;" % self.result())
          else:
              ExprNode.generate_disposal_code(self, code)



On 01/28/2014 01:47 PM, Syam Gadde wrote:
> Hi again,
>
> I'm exploring the typed memoryview feature with numpy and encountered a
> refcount error.  It seems to happen when I create a slice of a
> memoryview, and transpose it.  If I don't do both, I don't get the
> refcount error.  It happens in the error cleanup code.  I suspect that
> any error that causes the function to jump to the error cleanup code
> will do the same thing as the Exception below -- but that seemed the
> easiest way to demonstrate the problem. However, if there truly is an
> error in my code, please let me know! Thanks,
>
> -syam
>
> # BEGIN CODE
> import numpy
> cimport numpy
>
> class MyException(Exception):
>       def __init__(self):
>           Exception.__init__(self)
>
> def testfunc():
>      a = numpy.arange(12).reshape([3,4])
>
>      cdef numpy.int_t[:,:] a_view = a
>
>      ## here is a slice followed by a transpose
>      cdef numpy.int_t[:,:] b = a_view[:,:].T
>      ## same thing happens with a more realistic slicing:
>      #cdef numpy.int_t[:,:] b = a_view[1:2,:].T
>      ## however, there is no error if I do this instead:
>      #cdef numpy.int_t[:,:] b = a_view.T
>      ## also no error if I do this instead:
>      #cdef numpy.int_t[:,:] b = a_view[:,:]
>
>      # The exception below is just to force the function to abort
>      # and run the extraneous __PYX_XDEC_MEMVIEW in the error
>      # cleanup code:
>      #   Fatal Python error: Acquisition count is 0 (line XXXX)
>      # Comment out this exception and we don't get the error
>      raise MyException()
>
> try:
>       testfunc()
> except MyException:
>       pass
> # END CODE
>
>
>
> _______________________________________________
> cython-devel mailing list
> cython-devel at python.org
> https://mail.python.org/mailman/listinfo/cython-devel
>