[Python-checkins] python/dist/src/Python thread.c,2.53,2.54

[tim_one at users.sourceforge.net]

Update of /cvsroot/python/python/dist/src/Python
In directory sc8-pr-cvs1.sourceforge.net:/tmp/cvs-serv22438/Python

Modified Files:
	thread.c 
Log Message:
find_key():  This routine wasn't thread-correct, and accounts for the
release-build failures noted in bug 1041645.

This is a critical bugfix.  I'm not going to backport it, though (no time).


Index: thread.c
===================================================================
RCS file: /cvsroot/python/python/dist/src/Python/thread.c,v
retrieving revision 2.53
retrieving revision 2.54
diff -u -d -r2.53 -r2.54
--- thread.c	9 Oct 2004 22:33:09 -0000	2.53
+++ thread.c	10 Oct 2004 01:58:44 -0000	2.54
@@ -209,6 +209,15 @@
  * So when value==NULL, this acts like a pure lookup routine, and when
  * value!=NULL, this acts like dict.setdefault(), returning an existing
  * mapping if one exists, else creating a new mapping.
+ *
+ * Caution:  this used to be too clever, trying to hold keymutex only
+ * around the "p->next = keyhead; keyhead = p" pair.  That allowed
+ * another thread to mutate the list, via key deletion, concurrent with
+ * find_key() crawling over the list.  Hilarity ensued.  For example, when
+ * the for-loop here does "p = p->next", p could end up pointing at a
+ * record that PyThread_delete_key_value() was concurrently free()'ing.
+ * That could lead to anything, from failing to find a key that exists, to
+ * segfaults.  Now we lock the whole routine.
  */
 static struct key *
 find_key(int key, void *value)
@@ -216,22 +225,25 @@
 	struct key *p;
 	long id = PyThread_get_thread_ident();
 
+	PyThread_acquire_lock(keymutex, 1);
 	for (p = keyhead; p != NULL; p = p->next) {
 		if (p->id == id && p->key == key)
-			return p;
+			goto Done;
+	}
+	if (value == NULL) {
+		assert(p == NULL);
+		goto Done;
 	}
-	if (value == NULL)
-		return NULL;
 	p = (struct key *)malloc(sizeof(struct key));
 	if (p != NULL) {
 		p->id = id;
 		p->key = key;
 		p->value = value;
-		PyThread_acquire_lock(keymutex, 1);
 		p->next = keyhead;
 		keyhead = p;
-		PyThread_release_lock(keymutex);
 	}
+ Done:
+	PyThread_release_lock(keymutex);
 	return p;
 }