CRITICAL PATCH!

We occasionally received reports from people getting "invalid tstate"
crashes (this is a fatal error in PyThreadState_Delete()).  Finally
several people were able to reproduce it reliably and Tim Peters
discovered that there is a race condition when multiple threads are
calling this function without holding the global interpreter lock (the
function may be called without holding that).

Solved the race condition by adding a lock around the mutating uses of
interp->tstate_head.  Tim and Jonathan Giddy have run tests that make
it likely that this fixes the crashes -- although Tim hasn't heard
from the person who reported the original problem.

1 files changed, 23 insertions, 5 deletions

diff --git a/Python/pystate.c b/Python/pystate.c
index 338c0c3..b62fe7c 100644
--- a/Python/pystate.c
+++ b/Python/pystate.c
@@ -40,6 +40,18 @@ PERFORMANCE OF THIS SOFTWARE.
 }
 
 
+#ifdef WITH_THREAD
+#include "pythread.h"
+static PyThread_type_lock head_mutex = NULL; /* Protects interp->tstate_head */
+#define HEAD_INIT() (head_mutex || (head_mutex = PyThread_allocate_lock()))
+#define HEAD_LOCK() PyThread_acquire_lock(head_mutex, WAIT_LOCK)
+#define HEAD_UNLOCK() PyThread_release_lock(head_mutex)
+#else
+#define HEAD_INIT() /* Nothing */
+#define HEAD_LOCK() /* Nothing */
+#define HEAD_UNLOCK() /* Nothing */
+#endif
+
 static PyInterpreterState *interp_head = NULL;
 
 PyThreadState *_PyThreadState_Current = NULL;
@@ -51,6 +63,7 @@ PyInterpreterState_New()
 	PyInterpreterState *interp = PyMem_NEW(PyInterpreterState, 1);
 
 	if (interp != NULL) {
+		HEAD_INIT();
 		interp->modules = NULL;
 		interp->sysdict = NULL;
 		interp->builtins = NULL;
@@ -70,8 +83,10 @@ PyInterpreterState_Clear(interp)
 	PyInterpreterState *interp;
 {
 	PyThreadState *p;
+	HEAD_LOCK();
 	for (p = interp->tstate_head; p != NULL; p = p->next)
 		PyThreadState_Clear(p);
+	HEAD_UNLOCK();
 	ZAP(interp->modules);
 	ZAP(interp->sysdict);
 	ZAP(interp->builtins);
@@ -82,12 +97,11 @@ static void
 zapthreads(interp)
 	PyInterpreterState *interp;
 {
-	PyThreadState *p, *q;
-	p = interp->tstate_head;
-	while (p != NULL) {
-		q = p->next;
+	PyThreadState *p;
+	/* No need to lock the mutex here because this should only happen
+	   when the threads are all really dead (XXX famous last words). */
+	while ((p = interp->tstate_head) != NULL) {
 		PyThreadState_Delete(p);
-		p = q;
 	}
 }
 
@@ -139,8 +153,10 @@ PyThreadState_New(interp)
 		tstate->sys_profilefunc = NULL;
 		tstate->sys_tracefunc = NULL;
 
+		HEAD_LOCK();
 		tstate->next = interp->tstate_head;
 		interp->tstate_head = tstate;
+		HEAD_UNLOCK();
 	}
 
 	return tstate;
@@ -185,6 +201,7 @@ PyThreadState_Delete(tstate)
 	interp = tstate->interp;
 	if (interp == NULL)
 		Py_FatalError("PyThreadState_Delete: NULL interp");
+	HEAD_LOCK();
 	for (p = &interp->tstate_head; ; p = &(*p)->next) {
 		if (*p == NULL)
 			Py_FatalError(
@@ -193,6 +210,7 @@ PyThreadState_Delete(tstate)
 			break;
 	}
 	*p = tstate->next;
+	HEAD_UNLOCK();
 	PyMem_DEL(tstate);
 }