[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.

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?

To maintain the current semantics, the state must be left *exactly* as it
was when we did the PyAutoThreadState_Ensure().  This would involve keeping
state for each nested AutoThreadState call, which is a PITA, and I believe
unnecessary. PyAutoThreadState_Release() can obviously not unconditionally
release the lock, else it will not be held by SomeCallback() once the call
to SomeOtherFunc() returns.

The semantics I hope to implement are that the Python GIL is only released
when the *last* nested AutoThreadState call on a thread completes.  This
should also help perf, as we can avoid a number of lock transitions.

So, if we consider an extension module:
PyObject *MyExtFunc(PyObject *args, PyObject *self)
{
   ...
   Py_BEGIN_ALLOW_THREADS
   // note the more common case is that "SomeCallBack" is
   // invoked *indirectly* via the function being called - but
   // for the sake of demo, let's assume a direct call.
   SomeCallback();
   Py_END_ALLOW_THREADS
}


When SomeCallBack() completes, the PyAutoThreadState_Release() call realizes
that this is *not* the bottom of the stack for this thread, so does not
bother to release the lock.  Thus, when Py_END_ALLOW_THREADS is hit, we find
that the current thread still has the lock.

However, if "SomeCallBack()" had been called indirectly on another thread,
then PyAutoThreadState_Release() *would* release the lock, as that call will
be the final _Release() for that thread.

...

> If the issue is that there is (indeed very likely) code out there that
> calls PyEval_RestoreThread() rather than using the macro, would it be
> possible to change PyEval_RestoreThread() so that it acts the same as
> PyEval_EnsureThread()?

Yes, this is probably best, but means the doc for PyEval_RestoreThread would
change - in particular:

: If the lock has been created, the current thread must not have acquired
it,
: otherwise deadlock ensues.

However, as we are lossening this up, I see no real problem.

> Alas, no time to review code this week. :-(

No problem - thanks for helping me ensure I stay on track.

Mark.