[Cython] Error handling during buffer reassignment

Stefan Behnel stefan_ml at behnel.de
Sat Sep 9 06:03:19 EDT 2017

Robert Bradshaw schrieb am 08.09.2017 um 06:36:
> On Wed, Sep 6, 2017 at 3:53 AM, Stefan Behnel wrote:
>> consider this code:
>>     cdef np.ndarray[int32, ndim=1] a
>>     a = new_int32_buffer()
>>     a = new_float32_buffer()
>> This fails during the second assignment, but only during validation, after
>> unpacking the buffer. The code that handles this case is generated here:
>> https://github.com/cython/cython/blob/c95ca9f21a3524718a83c3415bb7102a508154be/Cython/Compiler/Buffer.py#L376
>> Note the twist where it says:
>>         # If acquisition failed, attempt to reacquire the old buffer
>>         # before raising the exception.
>> I faintly remember a discussion about that case (apparently back in 2008),
>> but can't find it in my mail archive. Might have been off-list at the time.
>> Question: Instead of re-acquiring the buffer (and thus faking the
>> non-assignment), wouldn't it be better to acquire the new buffer in a temp,
>> and only overwrite the old "Py_buffer" if all is fine with it?
> That makes a lot more sense to me, assuming overwriting is cheap enough.

It's just a Py_buffer struct copy.

> Can an error happen in releasing the old value?

No, releasing buffers is guaranteed to succeed (or rather, to not fail).

While working on this, I noticed that there is a difference. Previously,
the acquire-release order was:

   1) acquire initial buffer
   2) on reassign, release old buffer
   3) acquire new buffer
   4) on failure, re-aqcuire old buffer

Now the order is:

   1) acquire initial buffer
   2) on reassign, acquire new buffer
   3) on success, release old buffer

Note that the original code never holds two buffers at the same time. I
think the old way might have been optimised for that, i.e. it expected
success and tried to minimise the potential resource usage, assuming that
acquiring and holding a buffer might bind resources of the owner.

It's unclear to me whether that assumption holds, but it makes me more
hesitant to change the currect implementation.


More information about the cython-devel mailing list