[Python-Dev] Change definition of Py_END_ALLOW_THREADS?

[Mark Hammond]

> > > > However, to do this auto thread state work properly, I will
> > > > need to change the definition of Py_END_ALLOW_THREADS.
> > > > Specifically, Py_END_ALLOW_THREADS needs to be able to
> > > > handle the fact that the lock may or may not be held
> > > > when it is called (whereas now, the lock must *not* be held)
> > >
> > > Can you explain how the current thread, which explicitly
> > > released the
> > > lock in Py_BEGIN_ALLOW_THREADS, could end up owning the lock when
> > > Py_END_ALLOW_THREADS is entered?
> >
> > The short answer is that the current thread releases the lock (via
> > Py_BEGIN_ALLOW_THREAD), then makes the call it needs to.  This call
> > may synchronously call back into Python code, acquiring the lock.
> > Currently, the rule is that the callback must also release the lock
> > when done.  However, I am hoping to change things so that the
> > callback code need *not* re-release the lock when on the same
> > thread.
>
> Heh?  The point of Py_BEGIN_ALLOW_THREAD is to allow *other* threads
> to run, while the current thread engages in actions that could block
> indefinitely.  The code between BEGIN and END can cause any number of
> callbacks to run.  If one of those callbacks grabs the lock and
> doesn't release it, no other thread will be able to run, even if a
> later callback causes the current thread to block waiting for an
> external event!

This is true, but still true in many regards today.  Currently, between
BEGIN/END, a callback can still acquire the lock, leaving us in the same
position.  I am proposing that this lock need not be re-released.  In both
cases, there are periods of time where the current thread *does* hold the
lock between BEGIN/END pairs - I am simply increasing that time - but not to
the entire time.  Indeed, in the vast majority of cases where BEGIN/END is
currently used, there will be *no* change at all.

I agree I am on thin ice here, but let's move on for now.

> > The longer answer is to *why* I want to make that change:
> >
> > Consider:
> >
> > void SomeCallback()
> > {
> > 	PyAutoThreadState_Ensure();
> >       ... do some Python API stuff.
> >       SomeOtherFunc(); // my helper function.
> >       ... more Python stuff.
> >       PyAutoThreadState_Release();
> > }
> >
> > Now, consider SomeOtherFunc():
> >
> > void SomeOtherFunc()
> > {
> > 	// This function also needs to use the Python API
> > 	// but is called from many many places, where the
> > 	// Python context is not always known.
> > 	// So use the auto-thread-state
> > 	PyAutoThreadState_Ensure();
> > 	.. use the api
> > 	PyAutoThreadState_Release();
> > }
> >
> > As you can see, we have *nested* calls to
> PyAutoThreadState_Release().
> > The question is, in what state should PyAutoThreadState_Release()
> > leave Python?
>
> This suggests that the GIL needs to become a reentrant lock.

I don't think that solves our problem.

Consider a thread:

void SomeThreadEntryPoint()
  -> Acquire GIL
  -> Make some call into the Python API.
     -> Python code calls extension module fn.
        -> Extension module does Py_BEGIN_ALLOW_THREADS
...
  -> Release GIL

In this case, the Py_BEGIN_ALLOW_THREADS would release the GIL - but being a
reentrant lock, the lock itself would not actually be released, as
SomeThreadEntryPoint() has already acquired the same lock on the same
thread.

Thus, in this case, using a reentrant lock would *never* release the GIL in
Py_BEGIN_ALLOW_THREADS.  Indeed, from what I can see, the GIL will *never*
be released until SomeThreadEntryPoint terminates.  Current Python
thread-switch semantics assume that a GIL release is truly a lock release.

...
> Yes, releasing on the last call makes sense -- that's a
> reentrant lock.

Yes, but I am not sure a reentrant lock is suitable for the entire
thread-state API - I was hoping it would just be suitable for this
AutoThreadState API.  Now I am just confused :)

I need to ponder this some more, but it seems to me a reentrant lock is of
no use, as once a thread owns the lock, another thread can *never* acquire
it until the *final* matched release by the owning thread - and in some
cases that will be the entire lifetime of the thread.

Getting-harder-by-the-minute ly,

Mark.