Subject: proposed fixes for PRs 20256, 24241, 25722, 26096 -- libpthread assertions
To: None <tech-userlevel@netbsd.org>
From: Chuck Silvers <chuq@chuq.com>
List: tech-userlevel
Date: 10/09/2005 09:06:56
--J2SCkAp4GZ/dPZZf
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

hi,

the attached patch adds checks in three libpthread functions for
the case where these functions are called before pthread_create()
(and thus before pthread__start()) and calls pthread__start() in those
cases to avoid triggering assertions in the library.  in one of these
(pthread_mutex_lock()), the application will just proceed to deadlock
(since the thread will wait for a lock that it already owns), but that's
better than printing a message that looks like a bug in the library.
I had already closed the two PRs about this since the indication in
one of them was that the message was clear that the problem was in
the application, but later I tried it and saw that the message was
really about an internal error in the library.

the other cases (pthread_rwlock_timedwrlock() and sem_wait()) are
actually not deadlocks, since the thread will continue after the
timeout expires in the first case and after a signal wakes up
the thread in the second case.

anyone have any objections to this?

-Chuck

--J2SCkAp4GZ/dPZZf
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="diff.missing-start"

Index: pthread.c
===================================================================
RCS file: /cvsroot/src/lib/libpthread/pthread.c,v
retrieving revision 1.42
diff -u -p -r1.42 pthread.c
--- pthread.c	1 Jul 2005 12:35:18 -0000	1.42
+++ pthread.c	9 Oct 2005 15:58:57 -0000
@@ -237,7 +237,7 @@ pthread__child_callback(void)
 	pthread__started = 0;
 }
 
-static void
+void
 pthread__start(void)
 {
 	pthread_t self, idle;
@@ -548,7 +548,8 @@ pthread_exit(void *retval)
 	int nt;
 
 	self = pthread__self();
-	SDPRINTF(("(pthread_exit %p) Exiting (status %p, flags %x, cancel %d).\n", self, retval, self->pt_flags, self->pt_cancel));
+	SDPRINTF(("(pthread_exit %p) status %p, flags %x, cancel %d\n",
+		  self, retval, self->pt_flags, self->pt_cancel));
 
 	/* Disable cancellability. */
 	pthread_spinlock(self, &self->pt_flaglock);
@@ -597,6 +598,7 @@ pthread_exit(void *retval)
 		pthread_spinlock(self, &pthread__deadqueue_lock);
 		PTQ_INSERT_HEAD(&pthread__deadqueue, self, pt_allq);
 		pthread__block(self, &pthread__deadqueue_lock);
+		SDPRINTF(("(pthread_exit %p) walking dead dead\n", self));
 	} else {
 		self->pt_state = PT_STATE_ZOMBIE;
 		/* Note: name will be freed by the joiner. */
@@ -614,6 +616,7 @@ pthread_exit(void *retval)
 		 */
 		pthread__sched_sleepers(self, &self->pt_joiners);
 		pthread__block(self, &self->pt_join_lock);
+		SDPRINTF(("(pthread_exit %p) walking dead zombie\n", self));
 	}
 
 	/*NOTREACHED*/
Index: pthread_int.h
===================================================================
RCS file: /cvsroot/src/lib/libpthread/pthread_int.h,v
retrieving revision 1.31
diff -u -p -r1.31 pthread_int.h
--- pthread_int.h	26 Feb 2005 20:33:06 -0000	1.31
+++ pthread_int.h	9 Oct 2005 15:58:58 -0000
@@ -274,6 +274,7 @@ pthread_t pthread__next(pthread_t self);
 
 int	pthread__stackalloc(pthread_t *t);
 void	pthread__initmain(pthread_t *t);
+void	pthread__start(void);
 
 void	pthread__sa_start(void);
 void	pthread__sa_recycle(pthread_t old, pthread_t new);
Index: pthread_mutex.c
===================================================================
RCS file: /cvsroot/src/lib/libpthread/pthread_mutex.c,v
retrieving revision 1.19
diff -u -p -r1.19 pthread_mutex.c
--- pthread_mutex.c	16 Jul 2005 23:14:53 -0000	1.19
+++ pthread_mutex.c	9 Oct 2005 15:58:58 -0000
@@ -189,10 +189,16 @@ static int
 pthread_mutex_lock_slow(pthread_mutex_t *mutex)
 {
 	pthread_t self;
+	extern int pthread__started;
 
 	pthread__error(EINVAL, "Invalid mutex",
 	    mutex->ptm_magic == _PT_MUTEX_MAGIC);
 
+	if (pthread__started == 0) {
+		pthread__start();
+		pthread__started = 1;
+	}
+
 	self = pthread__self();
 
 	PTHREADD_ADD(PTHREADD_MUTEX_LOCK_SLOW);
Index: pthread_rwlock.c
===================================================================
RCS file: /cvsroot/src/lib/libpthread/pthread_rwlock.c,v
retrieving revision 1.11
diff -u -p -r1.11 pthread_rwlock.c
--- pthread_rwlock.c	9 Jan 2005 01:57:38 -0000	1.11
+++ pthread_rwlock.c	9 Oct 2005 15:58:58 -0000
@@ -248,6 +248,7 @@ pthread_rwlock_timedrdlock(pthread_rwloc
 	struct pthread_rwlock__waitarg wait;
 	struct pt_alarm_t alarm;
 	int retval;
+
 #ifdef ERRORCHECK
 	if ((rwlock == NULL) || (rwlock->ptr_magic != _PT_RWLOCK_MAGIC))
 		return EINVAL;
@@ -258,8 +259,8 @@ pthread_rwlock_timedrdlock(pthread_rwloc
 	    (abs_timeout->tv_nsec < 0) ||
 	    (abs_timeout->tv_sec < 0))
 		return EINVAL;
+
 	self = pthread__self();
-	
 	pthread_spinlock(self, &rwlock->ptr_interlock);
 #ifdef ERRORCHECK
 	if (rwlock->ptr_writer == self) {
@@ -316,8 +317,10 @@ pthread_rwlock_timedwrlock(pthread_rwloc
 {
 	struct pthread_rwlock__waitarg wait;
 	struct pt_alarm_t alarm;
-	int retval;
 	pthread_t self;
+	int retval;
+	extern int pthread__started;
+
 #ifdef ERRORCHECK
 	if ((rwlock == NULL) || (rwlock->ptr_magic != _PT_RWLOCK_MAGIC))
 		return EINVAL;
@@ -328,8 +331,13 @@ pthread_rwlock_timedwrlock(pthread_rwloc
 	    (abs_timeout->tv_nsec < 0) ||
 	    (abs_timeout->tv_sec < 0))
 		return EINVAL;
+
+	if (pthread__started == 0) {
+		pthread__start();
+		pthread__started = 1;
+	}
+
 	self = pthread__self();
-	
 	pthread_spinlock(self, &rwlock->ptr_interlock);
 #ifdef ERRORCHECK
 	if (rwlock->ptr_writer == self) {
Index: sem.c
===================================================================
RCS file: /cvsroot/src/lib/libpthread/sem.c,v
retrieving revision 1.7
diff -u -p -r1.7 sem.c
--- sem.c	24 Nov 2003 23:54:13 -0000	1.7
+++ sem.c	9 Oct 2005 15:58:58 -0000
@@ -286,6 +286,7 @@ int
 sem_wait(sem_t *sem)
 {
 	pthread_t self;
+	extern int pthread__started;
 
 #ifdef ERRORCHECK
 	if (sem == NULL || *sem == NULL || (*sem)->usem_magic != USEM_MAGIC) {
@@ -301,6 +302,11 @@ sem_wait(sem_t *sem)
 		return (_ksem_wait((*sem)->usem_semid));
 	}
 
+	if (pthread__started == 0) {
+		pthread__start();
+		pthread__started = 1;
+	}
+
 	for (;;) {
 		pthread_spinlock(self, &(*sem)->usem_interlock);
 		pthread_spinlock(self, &self->pt_statelock);

--J2SCkAp4GZ/dPZZf--