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--