[Cython] Recent bugs in generators

[Stefan Behnel]

Vitja Makarov, 20.04.2011 12:51:
> 2011/4/20 Stefan Behnel<stefan_ml at behnel.de>:
>> Vitja Makarov, 20.04.2011 11:50:
>>>
>>> 2011/4/20 Stefan Behnel:
>>>>
>>>> Vitja Makarov, 20.04.2011 10:26:
>>>>>
>>>>> 2011/4/18 Stefan Behnel:
>>>>>>
>>>>>> generators have their own lifetime frame in CPython, and
>>>>>> exceptions don't leak from that. So, whenever it's the generator (or
>>>>>> code
>>>>>> called by it) that raises an exception, that must be kept local to the
>>>>>> generator.
>>>>>
>>>>> Please review:
>>>>>
>>>>>
>>>>> https://github.com/vitek/cython/commit/73014aaed10b82a3f632d7f86212f86280c55858
>>>>>
>>>>> I've added __Pyx_Generator_SwapExceptions() method and call it right
>>>>> before resume switch and before return from yield. It swaps generator
>>>>> exception state with thread local.
>>>>
>>>> Looks good to me. I assume this fixes the problem? Then please push it
>>>> into
>>>> mainline.
>>>
>>> old pull request is still there ;)
>>>
>>> https://github.com/cython/cython/pull/25
>>>
>>> Does __Pyx_ExceptionReset() steal references to args, so they should
>>> not be decrefed later?
>>
>> Hmm, good call. The refcounting looks correct over the two function calls,
>> but it would be nicer if it could be avoided instead. We are really just
>> swapping around pointers here, no refcounting is needed.
>>
>> I think it's worth using a dedicated utility function for this.
>>
>
> Do you mean new utility call __Pyx_ExceptionSwap()?

Yes. I think it doesn't even have to be specific to the generator code. 
Just pass in "&gen->exc_type" etc.


>> Oh, and one more thing: what should be done when exiting the generator
>> normally? Would that just raise GeneratorExit anyway, so that we can swallow
>> any original outer exception? I think there might be a problem with Python
>> 3, where the GeneratorExit should be raised in the context of the original
>> exception. Worth testing how CPython behaves here...
>>
>
>
> Right! It's better to wrap call to generator:
>
> ExceptionSwap();
> generator_body();
> ExceptionSwap();

Good idea.


> Ok, I'll take a look.

Cool. Thanks!

Stefan