[Python-Dev] threading (GilState) question

[Michael Hudson]

Nick Coghlan <ncoghlan at gmail.com> writes:

> Michael Hudson wrote:
>> Option 1) Call PyEval_ThreadsInitialized() in PyGilState_Release().
>> Non-invasive, but bleh.
>
> Tim rejected this option back when PyEval_ThreadsInitialized() was
> added to the API [1].

Well, not really.  The patch that was rejected was much larger than
any proposal of mine.  My option 1) is this:

--- pystate.c   09 Feb 2005 10:56:18 +0000      2.39
+++ pystate.c   07 Apr 2005 13:19:55 +0100      
@@ -502,7 +502,8 @@
          PyThread_delete_key_value(autoTLSkey);
          }
          /* Release the lock if necessary */
-         else if (oldstate == PyGILState_UNLOCKED)
-              PyEval_ReleaseThread(tcur);
+              else if (oldstate == PyGILState_UNLOCKED
+                       && PyEval_ThreadsInitialized())
+                       PyEval_ReleaseThread();
 }
 #endif /* WITH_THREAD */

> Gustavo was having a similar problem with pygtk, and the end result
> was to add the ThreadsInitialized API so that pygtk could make its
> own check without slowing down the default case in the core.

Well, Gustavo seemed to be complaining about the cost of the locking.
I'm complaining about crashes.

>> Option 2) Call PyEval_SaveThread() instead of
>> PyEval_ReleaseThread()[1] in PyGilState_Release().  This is my
>> favourite option (PyGilState_Ensure() calls PyEval_RestoreThread which
>> is PyEval_SaveThread()s "mate") and I guess you can distill this long
>> mail into the question "why doesn't PyGilState_Release do this
>> already?"

This option corresponds to this patch:

--- pystate.c           09 Feb 2005 10:56:18 +0000      2.39
+++ pystate.c           07 Apr 2005 13:24:33 +0100      
@@ -503,6 +503,6 @@
   }
   /* Release the lock if necessary */
   else if (oldstate == PyGILState_UNLOCKED)
-          PyEval_ReleaseThread(tcur);
+          PyEval_SaveThread();
 }
 #endif /* WITH_THREAD */

> See above. Although I'm now wondering about the opposite question: Why
> doesn't PyGilState_Ensure use PyEval_AcquireThread?

Well, that would make more sense than what we have now.  OTOH, I'd
*much* rather make the PyGilState functions more tolerant -- I thought
being vaguely easy to use was part of their point.

I fail to believe the patch associated with option 2) has any
detectable performance cost.

Cheers,
mwh

-- 
  People think I'm a nice guy, and the fact is that I'm a scheming,
  conniving bastard who doesn't care for any hurt feelings or lost
  hours of work if it just results in what I consider to be a better
  system.                                            -- Linus Torvalds