-
Matěj Laitl authored
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.
6795a2ba