SF bug 1061968: threads: segfault or Py_FatalError at exit

PyGILState_Ensure():  The fix in 2.4a3 for bug 1010677 reintroduced thread
shutdown race bug 225673.  Repaired by (once again) ensuring the GIL is
held whenever deleting a thread state.

Alas, there's no useful test case for this shy bug.  Four years ago, only
Guido could provoke it, on his box, and today only Armin can provoke it
on his box.  I've never been able to provoke it (but not for lack of
trying!).

This is a critical fix for 2.3.5 too, since the fix for 1010677 got
backported there already and so also reintroduced 225673.  I don't intend to
backport this fix.  For whoever (if anyone) does, there are other thread
fixes in 2.4 that need backporting too, and I bet they need to happen first
for this patch to apply cleanly.

diff --git a/Misc/NEWS b/Misc/NEWS
index 9d6996de3069ee19ffca24cca37b4ad8b6619ab6..8efa6d2e8bfe972b423083bfd3db2af21a0010f9 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -12,7 +12,10 @@ What's New in Python 2.4 (release candidate 1|beta 3)
 Core and builtins
 -----------------
 
-...
+- Bug 1061968:  Fixes in 2.4a3 to address thread bug 1010677 reintroduced
+  the years-old thread shutdown race bug 225673.  Numeric history lesson
+  aside, all bugs in all three reports are fixed now.
+
 
 Extension Modules
 -----------------

diff --git a/Python/pystate.c b/Python/pystate.c
index 9ac02499e54f54eb1a980b8e74d1f2d8a5b1a0b6..182560748fe432f74fb523429b77294075a86db9 100644 (file)
--- a/Python/pystate.c
+++ b/Python/pystate.c
@@ -483,25 +483,24 @@ PyGILState_Release(PyGILState_STATE oldstate)
        --tcur->gilstate_counter;
        assert(tcur->gilstate_counter >= 0); /* illegal counter value */
 
-       /* If we are about to destroy this thread-state, we must
-          clear it while the lock is held, as destructors may run
-       */
+       /* If we're going to destroy this thread-state, we must
+        * clear it while the GIL is held, as destructors may run.
+        */
        if (tcur->gilstate_counter == 0) {
                /* can't have been locked when we created it */
                assert(oldstate == PyGILState_UNLOCKED);
                PyThreadState_Clear(tcur);
+               /* Delete the thread-state.  Note this releases the GIL too!
+                * It's vital that the GIL be held here, to avoid shutdown
+                * races; see bugs 225673 and 1061968 (that nasty bug has a
+                * habit of coming back).
+                */
+               PyThreadState_DeleteCurrent();
+               /* Delete this thread from our TLS. */
+               PyThread_delete_key_value(autoTLSkey);
        }
-
        /* Release the lock if necessary */
-       if (oldstate == PyGILState_UNLOCKED)
+       else if (oldstate == PyGILState_UNLOCKED)
                PyEval_ReleaseThread(tcur);
-
-       /* Now complete destruction of the thread if necessary */
-       if (tcur->gilstate_counter == 0) {
-               /* Delete this thread from our TLS */
-               PyThread_delete_key_value(autoTLSkey);
-               /* Delete the thread-state */
-               PyThreadState_Delete(tcur);
-       }
 }
 #endif /* WITH_THREAD */