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 9d6996d..8efa6d2 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -12,7 +12,10 @@
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 9ac0249..1825607 100644
--- a/Python/pystate.c
+++ b/Python/pystate.c
@@ -483,25 +483,24 @@
--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);
- }
-
- /* Release the lock if necessary */
- 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 */
+ /* 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);
- /* Delete the thread-state */
- PyThreadState_Delete(tcur);
}
+ /* Release the lock if necessary */
+ else if (oldstate == PyGILState_UNLOCKED)
+ PyEval_ReleaseThread(tcur);
}
#endif /* WITH_THREAD */