Exceptions caught and handled in Cython are "leaked" into Python

[robertwb]

There is a problem where Cython is not properly handling tstate->exc_* in exception handlers.
The "global" exception (tstate->exc_*) does not get cleared properly in an exception handler that raises a different exception.

Starting from an example, put the following into a pyx_leak.pyx file:

def foo():
    try:
        raise TypeError
    except TypeError:
        raise ValueError

And then from .py (or the prompt), do this:

import pyx_leak
import sys

def bar():
    try:
        pyx_leak.foo()
    except ValueError:
        pass

bar()
print sys.exc_info()

When you run this, you'll see that after calling bar, the TypeError exception is still set as the "global" exception.
The inner exception should not be leaking its way out (with normal Python, sys.exc_info() should return all None).

I'm not entirely certain what the correct way to handle this is. Looking at the generated code:

      __Pyx_Raise(__pyx_builtin_ValueError, 0, 0);
      {__pyx_filename = __pyx_f[0]; __pyx_lineno = 5; __pyx_clineno = __LINE__; goto __pyx_L7_except_error;}
      __Pyx_DECREF(__pyx_t_1); __pyx_t_1 = 0;
      __Pyx_DECREF(__pyx_t_2); __pyx_t_2 = 0;
      __Pyx_DECREF(__pyx_t_3); __pyx_t_3 = 0;
      goto __pyx_L6_exception_handled;
    }
    __pyx_L7_except_error:;
    __Pyx_XDECREF(__pyx_save_exc_type);
    __Pyx_XDECREF(__pyx_save_exc_value);
    __Pyx_XDECREF(__pyx_save_exc_tb);
    goto __pyx_L1_error;
    __pyx_L6_exception_handled:;
    __Pyx_XGIVEREF(__pyx_save_exc_type);
    __Pyx_XGIVEREF(__pyx_save_exc_value);
    __Pyx_XGIVEREF(__pyx_save_exc_tb);
    __Pyx_ExceptionReset(__pyx_save_exc_type, __pyx_save_exc_value, __pyx_save_exc_tb);
    __pyx_L12_try_end:;
  }

  __pyx_r = Py_None; __Pyx_INCREF(Py_None);
  goto __pyx_L0;
  __pyx_L1_error:;
  __Pyx_XDECREF(__pyx_t_1);
  __Pyx_XDECREF(__pyx_t_2);
  __Pyx_XDECREF(__pyx_t_3);
  __Pyx_AddTraceback("pyx_leak2.foo");
  __pyx_r = NULL;
  __pyx_L0:;
  __Pyx_XGIVEREF(__pyx_r);
  __Pyx_FinishRefcountContext();
  return __pyx_r;
}

You can see that raising ValueError does a jump to __pyx_L7_except_error (and oddly, the next 4 lines are unreachable).
Then it jumps to __pyx_L1_error. I think this is the critical problem. If the __Pyx_ExceptionReset block of code was
executed, then I think (maybe) this problem would go away.

As a side note, I think this problem was introduced by the change mentioned in ticket http://trac.cython.org/ticket/228.
Previously, __Pyx_Raise would set tstate->exc_* to NULL (which in this case is the inner TypeError).
Technically, I don't think that's the correct behavior (doing the __Pyx_ExceptionReset after __Pyx_Raise seems more correct to me when I compare it to CPython's implementation).

This is a problem for long-lived Python functions that make calls into Cython code which raise exceptions.
The "global" exception stays live for much too long.

Just FYI, I've been researching this for a while. If you're curious, here's CPython's behavior in the exception handler:

PyTraceback_Here()
PyErr_Fetch() # curexc->local variable
set_exc_info()
    tstate->exc_type = None
    frame->f_exc_type = tstate->exc_type # Which is None
    tstate->exc_* = local variable (TypeError)
do_raise()
    PyErr_Restore()
        tstate->curexc_* = ValueError
PyTraceback_Here()
reset_exc_info()
    tstate->exc_* = frame->f_exc_* (which is None)
    frame->f_exc_* = NULL
pop frame

Compared to Cython's behavior:

__Pyx_AddTraceback()
__Pyx_GetException()    
    Move tstate->curexc_* to tstate->exc_* (which is TypeError)
__Pyx_Raise()
    __Pyx_ErrRestore()
        tstate->curexc_* = ValueError
__Pyx_AddTraceback()
# NOTE: Critical error here, tstate->exc_* is still TypeError
Return NULL

I may take a look at a fix for this next week, but I am completely unfamiliar with how the compiler works, so I may not have time for it. I would really appreciate if anyone who is familiar with this to give any pointers or thoughts.

Migrated from http://trac.cython.org/ticket/346

[robertwb]

scoder commented

The general problem with exceptions is two-fold:

1. Cython doesn't have real frames, so it cannot just set the old exception value on function exit. There's code that keeps pointers to the original exception during try blocks, so that it can be reset afterwards. That's been in there for a while now and appears to work pretty well.

2. Cython needs to support both Py2 and Py3 correctly, meaning that it must set the exception context appropriately in Py3 and set up the global exception context as expected - but without changing the semantics of exception handling for Cython code. The problem you describe above is one where Py2 and Py3 behave differently due to the exception context.

I generally vote for Py3 semantics of exception raising and handling in Cython code, but we must map that to Py2.x and Py3 correctly under the hood. I've been working on the details for ages but never got around to finishing it up. Any help is appreciated.

[robertwb]

scoder changed milestone from wishlist to 0.12
commented

see also http://trac.cython.org/ticket/228

[robertwb]

scoder changed owner from somebody to scoder
commented

[robertwb]

scoder changed resolution to fixed
status from new to closed
commented

Fix is implemented here:

http://hg.cython.org/cython-devel/rev/587be39d90f9