[Cython] GIL handling C code

[Stefan Behnel]

Stefan Behnel, 28.02.2012 20:58:
> 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.

Hmm, guess I got confused again...

So, the code is this:

"""
       /*finally:*/ {
          int __pyx_why;
          __pyx_why = 0; goto __pyx_L8;
          __pyx_L7: __pyx_why = 4; goto __pyx_L8;
          __pyx_L8:;
          #ifdef WITH_THREAD
          PyGILState_Release(__pyx_gilstate_save);
          #endif
          switch (__pyx_why) {
            case 4: goto __pyx_L4;
          }
        }
    }
  }
  /*finally:*/ {
    int __pyx_why;
    __pyx_why = 0; goto __pyx_L5;
    __pyx_L4: __pyx_why = 4; goto __pyx_L5;
    __pyx_L5:;
    #ifdef WITH_THREAD
    __pyx_gilstate_save = PyGILState_Ensure();
    #endif
    switch (__pyx_why) {
      case 4: goto __pyx_L1_error;
    }
  }

  goto __pyx_L0;
  __pyx_L1_error:;
  __Pyx_XDECREF(__pyx_t_1);
  __Pyx_XDECREF(__pyx_t_2);
  __Pyx_WriteUnraisable("with_gil.void_nogil_ignore_exception",
__pyx_clineno, __pyx_lineno, __pyx_filename);
  __pyx_L0:;
  #ifdef WITH_THREAD
  PyGILState_Release(__pyx_gilstate_save);
  #endif
}
"""

The first "finally" block is inside of the with-gil block, whereas the
second is outside. In the second, the GIL is reacquired, and I guess that's
for cleaning up temps in the error case, right? I see the problem that it
can't be acquired after jumping to the exit labels (error/return) in the
current code layout, but wouldn't it make sense to generate this code
instead (starting at the second finally clause):

"""
  /*finally:*/ {
    int __pyx_why;
    __pyx_why = 0; goto __pyx_L5;
    __pyx_L4: __pyx_why = 4; goto __pyx_L5;
    __pyx_L5:;
    switch (__pyx_why) {
      case 4: goto __pyx_L1_error;
    }
  }

  goto __pyx_L0;
  __pyx_L1_error:;
  #ifdef WITH_THREAD
  __pyx_gilstate_save = PyGILState_Ensure();
  #endif
  __Pyx_XDECREF(__pyx_t_1);
  __Pyx_XDECREF(__pyx_t_2);
  __Pyx_WriteUnraisable("with_gil.void_nogil_ignore_exception",
__pyx_clineno, __pyx_lineno, __pyx_filename);
  #ifdef WITH_THREAD
  PyGILState_Release(__pyx_gilstate_save);
  #endif
  return;   /* <- error return here! */
  __pyx_L0:;
}
"""

Meaning: split the return code paths and let them independently acquire the
GIL if needed? That would at least relieve the outer finally from having to
care about the GIL and having to rely on "someone" to declare the GIL
variable and eventually release it.

Or, alternatively, add an additional exit path to the finally block that
jumps to a label that acquires the GIL before going on to the original
label. Something along those lines...

Stefan