[Cython] memoryview slice transpose

[Syam Gadde]

My patch actually addressed two slightly different issues, both of which 
have to do with refcounts of transposed slices.  I'll address the first 
one below since that's the one that's not so obvious.

Here's the code that causes an "Acquisition count is 0" error:

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
    raise Exception()  # comment this out and you still get a refcount 
error at a different line

testfunc()


On 04/06/2014 06:12 AM, Stefan Behnel wrote:
> Syam Gadde, 27.03.2014 21:29:
>> One problem is that when a memoryview slice is decremented, the .memview
>> and .data fields are not set to NULL, and any error that causes a jump
>> to cleanup code will attempt to decref it again.
> Hmm, I can't see why the additional NULL setting would change anything.
> That's what the call to put_xdecref_memoryviewslice() already did that
> immediately precedes them (it actually calls "__Pyx_XDEC_MEMVIEW()" in
> MemoryView_C.c).

But only if the refcount of the memview goes to zero.Maybe that's what 
needs to change?

See the case below (look for the *****'s).  memviewslice objects are 
defined at the top of the function and on the stack, so they persist 
throughout the function.  Several memviewslice objects may share the 
same base memview object.  So if a memviewslice object finished doing 
what it needs to do, it needs to decrement the refcount of the memview 
object it's pointing to using __PYX_XDEC_MEMVIEW, *and* detach itself 
from the memview (that's the setting .memview = NULL part).  The reason 
the explicit detaching is important is that there are extra 
__PYX_XDEC_MEMVIEW() calls in the error cleanup code, presumably to 
decrement the reference counts for still attached memviews in the case 
there is an error/exception.  If the memview field is set to NULL 
whenever a slice is done with it, the extra calls to __PYX_XDEC_MEMVIEW 
are OK because it checks to see whether .memview is NULL before going 
forward.  But currently, if an error/exception occurs while the slice is 
still attached to the memview, you can end up with a refcount < 0.

If __PYX_XDEC_MEMVIEW did indeed set .memview to NULL on every call, 
then this wouldn't be necessary.  But I have not tried that to see if it 
breaks something else.


   /* "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             # <<<<<<<<<<<<<<
  *    raise Exception()
  *
  */
   __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);
   __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;
   if (unlikely(__pyx_memslice_transpose(&__pyx_t_7) == 0)) 
{__pyx_filename = __p
yx_f[0]; __pyx_lineno = 7; __pyx_clineno = __LINE__; goto __pyx_L1_error;}
   __PYX_XDEC_MEMVIEW(&__pyx_t_5, 1);//**************************
   __pyx_v_b = __pyx_t_7;
   __pyx_t_7.memview = NULL;
   __pyx_t_7.data = NULL;

   /* "testtranspose.pyx":8
  *    cdef numpy.int_t[:,:] a_view = a
  *    cdef numpy.int_t[:,:] b = a_view[:,:].T
  *    raise Exception()             # <<<<<<<<<<<<<<
  *
  * testfunc()
  */
   __pyx_t_1 = __Pyx_PyObject_Call(__pyx_builtin_Exception, 
__pyx_empty_tuple, NULL); if (unlikely(!__pyx_t_1)) {__pyx_filename = 
__pyx_f[0]; __pyx_lineno = 8; __pyx_clineno = __LINE__; goto 
__pyx_L1_error;}
   __Pyx_GOTREF(__pyx_t_1);
   __Pyx_Raise(__pyx_t_1, 0, 0, 0);
   __Pyx_DECREF(__pyx_t_1); __pyx_t_1 = 0;
   {__pyx_filename = __pyx_f[0]; __pyx_lineno = 8; __pyx_clineno = 
__LINE__; goto __pyx_L1_error;}

   /* "testtranspose.pyx":4
  * cimport numpy
  *
  * def testfunc():             # <<<<<<<<<<<<<<
  *    a = numpy.arange(12).reshape([3,4])
  *    cdef numpy.int_t[:,:] a_view = a
  */

   /* function exit code */
   __pyx_L1_error:;  // ****** exception causes a jump to here *****
   __Pyx_XDECREF(__pyx_t_1);
   __Pyx_XDECREF(__pyx_t_2);
   __Pyx_XDECREF(__pyx_t_3);
   __PYX_XDEC_MEMVIEW(&__pyx_t_4, 1);
   __PYX_XDEC_MEMVIEW(&__pyx_t_5, 1); //****************************
   __PYX_XDEC_MEMVIEW(&__pyx_t_7, 1);
   __Pyx_AddTraceback("testtranspose.testfunc", __pyx_clineno, 
__pyx_lineno, __pyx_filename);
   __pyx_r = NULL;
   __Pyx_XDECREF(__pyx_v_a);
   __PYX_XDEC_MEMVIEW(&__pyx_v_a_view, 1);
   __PYX_XDEC_MEMVIEW(&__pyx_v_b, 1);
   __Pyx_XGIVEREF(__pyx_r);
   __Pyx_RefNannyFinishContext();
   return __pyx_r;