Document the results of painful reverse-engineering of the "portable TLS"

code.

PyThread_set_key_value():  It's clear that this code assumes the passed-in
value isn't NULL, so document that it must not be, and assert that it
isn't.  It remains unclear whether existing callers want the odd semantics
actually implemented by this function.

1 files changed, 84 insertions, 6 deletions

diff --git a/Python/thread.c b/Python/thread.c
index 3985779..b1ddf53 100644
--- a/Python/thread.c
+++ b/Python/thread.c
@@ -142,26 +142,80 @@ void PyThread_init_thread(void)
    This code stolen from "thread_sgi.h", where it was the only
    implementation of an existing Python TLS API.
 */
-/*
- * Per-thread data ("key") support.
- */
+/* ------------------------------------------------------------------------
+Per-thread data ("key") support.
+
+Use PyThread_create_key() to create a new key.  This is typically shared
+across threads.
+
+Use PyThread_set_key_value(thekey, value) to associate void* value with
+thekey in the current thread.  Each thread has a distinct mapping of thekey
+to a void* value.  Caution:  if the current thread already has a mapping
+for thekey, value is ignored.
+
+Use PyThread_get_key_value(thekey) to retrieve the void* value associated
+with thekey in the current thread.  This returns NULL if no value is
+associated with thekey in the current thread.
+
+Use PyThread_delete_key_value(thekey) to forget the current thread's associated
+value for thekey.  PyThread_delete_key(thekey) forgets the values associated
+with thekey across *all* threads.
 
+While some of these functions have error-return values, none set any
+Python exception.
+
+None of the functions does memory management on behalf of the void* values.
+You need to allocate and deallocate them yourself.  If the void* values
+happen to be PyObject*, these functions don't do refcount operations on
+them either.
+
+The GIL does not need to be held when calling these functions; they supply
+their own locking.  This isn't true of PyThread_create_key(), though (see
+next paragraph).
+
+There's a hidden assumption that PyThread_create_key() will be called before
+any of the other functions are called.  There's also a hidden assumption
+that calls to PyThread_create_key() are serialized externally.
+------------------------------------------------------------------------ */
+
+/* A singly-linked list of struct key objects remembers all the key->value
+ * associations.  File static keyhead heads the list.  keymutex is used
+ * to enforce exclusion internally.
+ */
 struct key {
+	/* Next record in the list, or NULL if this is the last record. */
 	struct key *next;
+
+	/* The thread id, according to PyThread_get_thread_ident(). */
 	long id;
+
+	/* The key and its associated value. */
 	int key;
 	void *value;
 };
 
 static struct key *keyhead = NULL;
-static int nkeys = 0;
 static PyThread_type_lock keymutex = NULL;
-
+static int nkeys = 0;  /* PyThread_create_key() hands out nkeys+1 next */
+
+/* Internal helper.
+ * If the current thread has a mapping for key, the appropriate struct key*
+ * is returned.  NB:  value is ignored in this case!
+ * If there is no mapping for key in the current thread, then:
+ *     If value is NULL, NULL is returned.
+ *     Else a mapping of key to value is created for the current thread,
+ *     and a pointer to a new struct key* is returned; except that if
+ *     malloc() can't find room for a new struct key*, NULL is returned.
+ * 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.
+ */
 static struct key *
 find_key(int key, void *value)
 {
 	struct key *p;
 	long id = PyThread_get_thread_ident();
+
 	for (p = keyhead; p != NULL; p = p->next) {
 		if (p->id == id && p->key == key)
 			return p;
@@ -181,18 +235,27 @@ find_key(int key, void *value)
 	return p;
 }
 
+/* Return a new key.  This must be called before any other functions in
+ * this family, and callers must arrange to serialize calls to this
+ * function.  No violations are detected.
+ */
 int
 PyThread_create_key(void)
 {
+	/* All parts of this function are wrong if it's called by multiple
+	 * threads simultaneously.
+	 */
 	if (keymutex == NULL)
 		keymutex = PyThread_allocate_lock();
 	return ++nkeys;
 }
 
+/* Forget the associations for key across *all* threads. */
 void
 PyThread_delete_key(int key)
 {
 	struct key *p, **q;
+
 	PyThread_acquire_lock(keymutex, 1);
 	q = &keyhead;
 	while ((p = *q) != NULL) {
@@ -207,31 +270,46 @@ PyThread_delete_key(int key)
 	PyThread_release_lock(keymutex);
 }
 
+/* Confusing:  If the current thread has an association for key,
+ * value is ignored, and 0 is returned.  Else an attempt is made to create
+ * an association of key to value for the current thread.  0 is returned
+ * if that succeeds, but -1 is returned if there's not enough memory
+ * to create the association.  value must not be NULL.
+ */
 int
 PyThread_set_key_value(int key, void *value)
 {
-	struct key *p = find_key(key, value);
+	struct key *p;
+
+	assert(value != NULL);
+	p = find_key(key, value);
 	if (p == NULL)
 		return -1;
 	else
 		return 0;
 }
 
+/* Retrieve the value associated with key in the current thread, or NULL
+ * if the current thread doesn't have an association for key.
+ */
 void *
 PyThread_get_key_value(int key)
 {
 	struct key *p = find_key(key, NULL);
+
 	if (p == NULL)
 		return NULL;
 	else
 		return p->value;
 }
 
+/* Forget the current thread's association for key, if any. */
 void
 PyThread_delete_key_value(int key)
 {
 	long id = PyThread_get_thread_ident();
 	struct key *p, **q;
+
 	PyThread_acquire_lock(keymutex, 1);
 	q = &keyhead;
 	while ((p = *q) != NULL) {