[Cython] Recent bugs in generators

[Stefan Behnel]

Vitja Makarov, 20.04.2011 10:26:
> 2011/4/18 Stefan Behnel:
>> Vitja Makarov, 18.04.2011 06:38:
>>>
>>> 2011/4/18 Stefan Behnel:
>>>>
>>>> Vitja Makarov, 17.04.2011 17:57:
>>>>>
>>>>> 3. check_yield_in_exception()
>>>>
>>>> I added this because I found a failing pyregr test that uses it (testing
>>>> the
>>>> @contextmanager decorator).
>>>>
>>>>
>>>>> Cython calls __Pyx_ExceptionReset when except block is done, so when
>>>>> yield is there no exception reset is called.
>>>>>
>>>>> I'm not sure how to fix this.
>>>>
>>>> I'm not completely sure either.
>>>>
>>>>
>>>>> import sys
>>>>>
>>>>> def foo():
>>>>>      """
>>>>>      >>>      list(foo())
>>>>>      [<type 'exceptions.ValueError'>, None]
>>>>>      """
>>>>>      try:
>>>>>          raise ValueError
>>>>>      except ValueError:
>>>>>          yield sys.exc_info()[0]
>>>>>          yield sys.exc_info()[0] # exc_info is lost here
>>>>
>>>> I think (!), the difference here is that CPython actually keeps the
>>>> exception in the generator frame. We don't have a frame, so we have to
>>>> emulate it using the closure class. I guess we'll have to store away the
>>>> exception into the closure when we yield while an exception is being
>>>> handled, and restore it afterwards. Note: this is not the exception that
>>>> is
>>>> freshly *being* raised (the "_cur*" fields in the thread state), it's the
>>>> exception that *was* raised and is now being handled, i.e. the thread
>>>> state
>>>> fields without the "_cur", that are reflected by sys.exc_info().
>>>
>>> Interesting difference between py2 and py3:
>>>
>>> def foo():
>>>      try:
>>>          raise ValueError
>>>      except ValueError:
>>>          yield
>>>          raise
>>> list(foo())
>>>
>>>    File "xxx.py", line 7, in<module>
>>>      list(foo())
>>>    File "xxx.py", line 6, in foo
>>>      raise
>>> TypeError: exceptions must be old-style classes or derived from
>>> BaseException, not NoneType
>>>
>>> It seems that exception info is completely lost (tried 2.6, 2.7) and
>>> seems to be fixed in python3.
>>
>> Not surprising. The implementation is completely different in Py2 and Py3,
>> both in CPython and in Cython. It's actually much simpler in Cython under
>> Py3, due to better semantics and C-API support. That also implies that
>> there's much less Cython can do wrong in that environment. ;-)
>>
>>
>>> Btw exception info temps are already saved and restored between yields.
>>
>> Right, but the exc_info itself is not reset and recovered around the yield.
>> As I said above, 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.

Stefan