[Python-Dev] Change definition of Py_END_ALLOW_THREADS?

[Guido van Rossum]

>   As part of my "auto thread state" work, I am discovering some very
> interesting things - including the fact that pythread.h *already* defines a
> Python Thread Local Storage API, and that thread_sgi.h contains a safe,
> portable working implementation :)

Feel free to migrate that into thread.c with some #ifdef test so a
platform can offer something better.

> 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?

> Specifically, the changes would look like:
> * New function to determine if the current thread's threadstate is "current"
> * New function, PyEval_EnsureThread(), which is nearly identical to
> PyEval_RestoreThread(), except it only attempts the restore of the thread
> state if it is not already current.
> 
> The above 2 are fairly easy to implement in a thread-safe manner - if the
> thread only checks its *own* thread state is current, the result of that
> check will remain valid on this thread, even if another thread becomes
> current during processing.
> 
> This means that Py_END_ALLOW_THREADS then changes from:
> #define Py_END_ALLOW_THREADS  PyEval_RestoreThread(_save); \
> to:
> #define Py_END_ALLOW_THREADS  PyEval_EnsureThread(_save); \
> 
> Now, I understand that a complete blessing of this scheme will need more
> details - but I am concerned that *any* change to Py_END_ALLOW_THREADS will
> blow this scheme out of the water.  For example, the Win32 extensions have,
> in a few places, emulated Py_BEGIN_ALLOW_THREADS, and all of this code will
> need to change.  From my POV though, the only reason I needed these hacks in
> the first place was due to an incomplete thread state API, so such changes
> are worthwhile to me.  Other's mileage may vary.

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()?

> FWIW, I have a portable (ie, no windows specific code at all) "auto thread
> state" implementation fully working with the Win32all extensions.  These
> extensions do some funky things with threads and thread-states, and it all
> works perfectly.  However, it does rely on this Py_END_ALLOW_THREAD change.
> See the full patch at http://www.python.org/sf/684256

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

--Guido van Rossum (home page: http://www.python.org/~guido/)