[Cython] GIL handling C code

[mark florisson]

On 28 February 2012 21:38, Stefan Behnel <stefan_ml at behnel.de> wrote:
> 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?
>

Sure, that's fine with me.

>> (<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
> _______________________________________________
> cython-devel mailing list
> cython-devel at python.org
> http://mail.python.org/mailman/listinfo/cython-devel