[Cython] tp_clear() of buffer client objects (was: Re: test crash in Py3.2 (NumPy/memoryview/refcounting related?))

[mark florisson]

On 23 April 2012 09:42, Stefan Behnel <stefan_ml at behnel.de> wrote:
> Stefan Behnel, 23.04.2012 10:23:
>> mark florisson, 22.04.2012 16:41:
>>> On 22 April 2012 15:31, mark florisson wrote:
>>>> I think the problem here
>>>> is that a single memoryview object is traversed multiple times through
>>>> different traverse functions, and that the refcount doesn't match the
>>>> number of traverses. Indeed, the refcount is only one, as the actual
>>>> count is the acquisition count. So we shouldn't traverse the
>>>> memoryview objects in memoryview slices, i.e. not
>>>> _memoryviewslice.from_slice.memview. I'll come up with a commit
>>>> shortly, would you be willing to test it?
>>>
>>> BTW, tp_clear calls Py_CLEAR on Py_buffer.obj, shouldn't it call
>>> releasebuffer instead?
>>
>> Where is that? The memoryview class calls __Pyx_ReleaseBuffer() here.
>
> Ah, found it. Yes, tp_clear() would be called before __dealloc__() in
> reference cycles, so that's a problem. I'm not sure tp_clear should do
> something as (potentially) involved as freeing the buffer, but if the
> Py_buffer is owned by the object, then I guess it just has to do that.
> Otherwise, it would leak the buffer.
>
> The problem is that this also impacts user code, though, so a change might
> break code, e.g. when it needs to do some cleanup on its own before freeing
> the buffer. It would make the code more correct, sure, but it would still
> break it. Guess we have to take that route, though...
>
> Stefan
> _______________________________________________
> cython-devel mailing list
> cython-devel at python.org
> http://mail.python.org/mailman/listinfo/cython-devel

It also seems that tp_clear is only generated for object attributes,
and then it includes freeing the object of Py_buffers (so only a
Py_buffer attribute doesn't result in a tp_clear function). Finally,
tp_dealloc doesn't deallocate buffers either, which it should. So both
tp_clear and tp_dealloc should call release buffer (it can be called
multiple times on the same buffer).