On 22 September 2010 10:22, Stephane DROUARD <[email protected]> wrote:
> Hello,
>
> With the following code:
>
> test.pyx
> --------
> cdef extern from "foo.h":
> void foo() nogil except +
>
> def bar():
> with nogil:
> foo()
>
> foo.c
> -----
> void foo()
> {
> throw std::runtime_error("foo exception");
> }
>
> The following crashes:
> python -c "import test; test.bar()"
>
>
> Dumping at the code of bar() generated by Cython:
>
> { PyThreadState *_save;
> Py_UNBLOCK_THREADS
> /*try:*/ {
> try {foo();} catch(...) {__Pyx_CppExn2PyErr(); {__pyx_filename =
> __pyx_f[0]; __pyx_lineno = 6; __pyx_clineno = __LINE__; goto __pyx_L6;}}
> }
> /*finally:*/ {
> int __pyx_why;
> __pyx_why = 0; goto __pyx_L7;
> __pyx_L6: __pyx_why = 4; goto __pyx_L7;
> __pyx_L7:;
> Py_BLOCK_THREADS
> switch (__pyx_why) {
> case 4: goto __pyx_L1_error;
> }
> }
> }
>
> The problem is that __Pyx_CppExn2PyErr() accesses Python without the GIL.
>
Good catch.
> There are several ways of fixing this.
>
> 1/ The simplest, but not the most optimized one: acquire/release the GIL
> within __Pyx_CppExn2PyErr().
>
Why do you say (1) is not the most optimized one? (Sorry, I'm not good
at understanding issues with thread-based concurrency)
> 2/ acquire/release the GIL in the catch clause only when "with nogil":
> try {foo();} catch(...) {PyGILState_STATE state = PyGILState_Ensure();
> __Pyx_CppExn2PyErr(); PyGILState_Release(state); {__pyx_filename =
> __pyx_f[0]; __pyx_lineno = 6; __pyx_clineno = __LINE__; goto __pyx_L6;}}
>
How would this be different to (1) ?
> 3/ restore the thread in the catch clause:
> try {foo();} catch(...) { Py_BLOCK_THREADS __Pyx_CppExn2PyErr();
> {__pyx_filename = __pyx_f[0]; __pyx_lineno = 6; __pyx_clineno = __LINE__;
> goto __pyx_L6;}}
> }
> /*finally:*/ {
> int __pyx_why;
> Py_BLOCK_THREADS // <<<<<<<<<<<<<<<<
> __pyx_why = 0; goto __pyx_L7;
> __pyx_L6: __pyx_why = 4; goto __pyx_L7;
> __pyx_L7:;
> switch (__pyx_why) {
> case 4: goto __pyx_L1_error;
> }
> }
> }
>
And this one looks bad, I think you need: Py_BLOCK_THREADS
__Pyx_CppExn2PyErr(); Py_UNBLOCK_THREADS .. Am I right?
What about doing this (ignore the line endings, all should be
generated in a single line)
{ PyThreadState *_save;
try {
Py_UNBLOCK_THREADS
foo();
Py_BLOCK_THREADS
} catch(...) {
Py_BLOCK_THREADS
__Pyx_CppExn2PyErr();
}
Am I missing something? However, note that changing to this could be
not so easy...
--
Lisandro Dalcin
---------------
CIMEC (INTEC/CONICET-UNL)
Predio CONICET-Santa Fe
Colectora RN 168 Km 472, Paraje El Pozo
Tel: +54-342-4511594 (ext 1011)
Tel/Fax: +54-342-4511169
_______________________________________________
Cython-dev mailing list
[email protected]
http://codespeak.net/mailman/listinfo/cython-dev
