Nick Coghlan <ncoghlan@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