[Cython] GIL handling C code

Stefan Behnel stefan_ml at behnel.de
Tue Feb 28 22:38:31 CET 2012


mark florisson, 28.02.2012 22:19:
> On 28 February 2012 21:08, Stefan Behnel 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.
>>
>> 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?
> 
> It looks right to me, yeah.

Ok. I think this looks simple enough to go into the release, whereas any
more advanced cleanup and optimisations should have their time to mature.
Would you object?


> (<pedantic>I prefer to format bools
> directly with %d, as they are a subclass of int anyway</pedantic>).

I don't. It works when they are really booleans, but in Python, many things
that are "true" or "false" aren't actually of type bool. When it comes to
writing out (user visible) data, it's always best to make it clear what the
intended output is, instead of relying on 'implementation details' and data
of unknown sources (or different purposes, as in this case).


> I think in general the EnsureGILNode should have been mentioned in the
> code generation function of FuncDefNode, which makes it easier to
> figure out what is going on. The documentation is currently at the
> wrapping site in AnalyseDeclarationsTransform.

I'll add a comment.

Stefan


More information about the cython-devel mailing list