cython/cython — pull request #178 by strohel (2013-01-25 11:13:58+00:00)
Title: Fix calling an "except +" cpp function in a nogil function

For a source:
|cdef extern from "":
| cdef void foo_cpp() nogil except +
|
|cdef call_foo() nogil:
| foo_cpp()

We used to generate something like this to actually call foo_cpp() in call_foo():
|try{foo_cpp();}
|catch(...) {
| Py_BLOCK_THREADS; __Pyx_CppExn2PyErr(); Py_UNBLOCK_THREADS
| `code.error_goto(self.pos))`
|}

where Py_BLOCK_THREADS expands to "PyEval_RestoreThread(_save);".
__Pyx_CppExn2PyErr() (and alternatives, see SimpleCallNode) calls CPython A API
methods so it needs to be guarded in a nogil environment.

One problem is that "PyThreadState *_save" is only declared by "with nogil:"
block, so a .cpp file that doesn't compile is generated for the above code.

However, I think the real issue is that Py_(UN)BLOCK_THREADS is inappropriate
here, as it isn't allowed to be called recursively and is valid only directly
in a Py_BEGIN_ALLOW_THREADS ... Py_END_ALLOW_THREADS. IMO PyGILState_Ensure()
and PyGILState_Release() (through `code.put_ensure_gil()` and a friend) is the
correct thing to call here as it is allowed to be called recursively and
actually ensures the current thread can call CPython C API.

This patch does exactly this (and it breaks the generated code to multiple
lines as it would be way too long otherwise), plus it extends the
cpp_exceptions_nogil.pyx test with above example that doesn't compile with this
fix not applied.

Note that we explicitly pass declare_gilstate=True to put_ensure_gil(), as
PyGILState_Ensure() docs state that recursive calls to it must not share the
PyGILState_STATE. C++-style declaring a variable inside a block should be
no-problem here, as try{} .. catch() is obviously valid only in a C++ code.