Fix calling an "except +" cpp function in a nogil function
authorMatěj Laitl <matej@laitl.cz>
Fri, 25 Jan 2013 10:27:49 +0000 (11:27 +0100)
committerMatěj Laitl <matej@laitl.cz>
Fri, 25 Jan 2013 10:27:49 +0000 (11:27 +0100)
commit6eb0745f2a28cbf60bfbe4feb40f95ddb5b54c46
tree40cbdd639a07300957ac8933340c04ffbbbd2858
parent3eedf16510a888485b133a0acbafad920bd7480d
Fix calling an "except +" cpp function in a nogil function

For a source:
|cdef extern from "<foo>":
|    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.

--HG--
extra : transplant_source : %AA%F3%BDk%FE%F9%01%7F%8C%A4n%5E%DA4%97%A5%D9%AF%D60
Cython/Compiler/ExprNodes.py
tests/run/cpp_exceptions_nogil.pyx