PERFORCE change 73750 for review

[David Xu]

http://perforce.freebsd.org/chv.cgi?CH=73750

Change 73750 by davidxu at davidxu_alona on 2005/03/24 12:53:22

	tcb is now managed by rtld, caching it is not correct,
	otherwise we may get stale copy of tls data from previous
	dead thread, caching should be done in rtld if needed.

Affected files ...

.. //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_list.c#6 edit

Differences ...

==== //depot/projects/davidxu_thread/src/lib/libthread/thread/thr_list.c#6 (text+ko) ====

@@ -139,7 +139,7 @@
 _thr_alloc(struct pthread *curthread)
 {
 	struct pthread	*thread = NULL;
-	struct tcb	*tcb = NULL;
+	struct tcb	*tcb;
 
 	if (curthread != NULL) {
 		if (GC_NEEDED())
@@ -148,29 +148,29 @@
 			THR_LOCK_ACQUIRE(curthread, &free_thread_lock);
 			if ((thread = TAILQ_FIRST(&free_threadq)) != NULL) {
 				TAILQ_REMOVE(&free_threadq, thread, tle);
-				tcb = thread->tcb;
 				free_thread_count--;
 			}
 			THR_LOCK_RELEASE(curthread, &free_thread_lock);
 		}
 	}
-	if ((thread == NULL) &&
-	    ((thread = malloc(sizeof(struct pthread))) != NULL)) {
-		if (curthread) {
-			THR_LOCK_ACQUIRE(curthread, &tcb_lock);
-			tcb = _tcb_ctor(thread, 0 /* not initial tls */);
-			THR_LOCK_RELEASE(curthread, &tcb_lock);
-		} else {
-			tcb = _tcb_ctor(thread, 1 /* initial tls */);
-		}
-		if (tcb == NULL) {
-			free(thread);
-			thread = NULL;
-		}
+	if (thread == NULL) {
+		thread = malloc(sizeof(struct pthread));
+		if (thread == NULL)
+			return (NULL);
+	}
+	if (curthread != NULL) {
+		THR_LOCK_ACQUIRE(curthread, &tcb_lock);
+		tcb = _tcb_ctor(thread, 0 /* not initial tls */);
+		THR_LOCK_RELEASE(curthread, &tcb_lock);
+	} else {
+		tcb = _tcb_ctor(thread, 1 /* initial tls */);
 	}
-	if (thread) {
+	if (tcb != NULL) {
 		memset(thread, 0, sizeof(*thread));
 		thread->tcb = tcb;
+	} else {
+		thr_destroy(curthread, thread);
+		thread = NULL;
 	}
 	return (thread);
 }
@@ -183,10 +183,26 @@
 		free(thread->name);
 		thread->name = NULL;
 	}
+	/*
+	 * Always free tcb, as we only know it is part of RTLD TLS
+	 * block, but don't know its detail and can not assume how
+	 * it works, so better to avoid caching it here.
+	 */
+	if (curthread != NULL) {
+		THR_LOCK_ACQUIRE(curthread, &tcb_lock);
+		_tcb_dtor(thread->tcb);
+		THR_LOCK_RELEASE(curthread, &tcb_lock);
+	} else {
+		_tcb_dtor(thread->tcb);
+	}
+	thread->tcb = NULL;
 	if ((curthread == NULL) || (free_thread_count >= MAX_CACHED_THREADS)) {
 		thr_destroy(curthread, thread);
 	} else {
-		/* Add the thread to the free thread list. */
+		/*
+		 * Add the thread to the free thread list, this also avoids
+		 * pthread id is reused too quickly, may help some buggy apps.
+		 */
 		THR_LOCK_ACQUIRE(curthread, &free_thread_lock);
 		TAILQ_INSERT_TAIL(&free_threadq, thread, tle);
 		free_thread_count++;
@@ -195,15 +211,8 @@
 }
 
 static void
-thr_destroy(struct pthread *curthread, struct pthread *thread)
+thr_destroy(struct pthread *curthread __unused, struct pthread *thread)
 {
-	if (curthread) {
-		THR_LOCK_ACQUIRE(curthread, &tcb_lock);
-		_tcb_dtor(thread->tcb);
-		THR_LOCK_RELEASE(curthread, &tcb_lock);
-	} else {
-		_tcb_dtor(thread->tcb);
-	}
 	free(thread);
 }