On 28 February 2012 21:08, Stefan Behnel <[email protected]> wrote: > mark florisson, 28.02.2012 21:20: >> On 28 February 2012 19:58, Stefan Behnel wrote: >>> mark florisson, 28.02.2012 16:35: >>>> Basically, the cleanup code only needs a matching release because the >>>> corresponding acquire is in EnsureGILNode, which wraps the function >>>> body in case of a nogil function with a 'with gil' block. Any changes >>>> to the conditions in FuncDefNode will have to be reflected by the code >>>> that does that wrapping. Changing the refnanny macro for the cleanup >>>> code will not avail anything, as the GIL is already ensured. >>> >>> Regarding the "with gil" code, ISTM that the "finally" code in the with_gil >>> test is being duplicated. I noticed this when I moved the refnanny's GIL >>> state into a block local variable and that broke the C code. Basically, the >>> with-gil block had declared the variable in its own block, but was then >>> trying to access that variable in a second finally clause, further down and >>> outside of the with-gil block. >>> >>> Looks like a bug to me. >> >> That's not a bug, that's how it is implemented. At setup, a variable >> __pyx_gilstate_save is declared, which is also used for teardown. Any >> with GIL blocks use C block scoping to do the same thing. The with >> block is itself a try/finally, so you get a try/finally wrapped in a >> try/finally. The code uses try/finally for it's way of trapping >> control flow, allowing some code to execute and resuming control flow >> afterwards. > > Ok, so what really got me confused is that the code used the variable > "acquire_gil_for_refnanny_only" in places that didn't have anything to do > with the refnanny. Similarly, "acquire_gil_for_var_decls_only" was used for > cleanup even though the GIL had already been released if this flag was set, > way before the end of the function.
Yeah those names weren't very apt, they are useful for the setup code but don't make sense for the teardown code :) > I think I fixed both issues in the patch I attached. At least, it still > passes the gil related tests and doesn't raise any C compiler warnings > about the GIL state variable being unused. > > Does this look about right? > > Stefan > > _______________________________________________ > cython-devel mailing list > [email protected] > http://mail.python.org/mailman/listinfo/cython-devel > _______________________________________________ cython-devel mailing list [email protected] http://mail.python.org/mailman/listinfo/cython-devel
