Convert thrsleep() to an absolute timeout with clockid to eliminate a

race condition and prep for later support of pthread_condattr_setclock()

"get it in" deraadt@, tedu@, cheers by others

10 files changed, 105 insertions, 68 deletions

diff --git a/lib/librthread/rthread.c b/lib/librthread/rthread.c
index 8b9a4be6648..9c9f4a6a37f 100644
--- a/lib/librthread/rthread.c
+++ b/lib/librthread/rthread.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rthread.c,v 1.41 2009/11/27 19:42:24 guenther Exp $ */
+/*	$OpenBSD: rthread.c,v 1.42 2009/11/27 19:45:54 guenther Exp $ */
 /*
  * Copyright (c) 2004,2005 Ted Unangst <tedu@openbsd.org>
  * All Rights Reserved.
@@ -253,7 +253,7 @@ pthread_join(pthread_t thread, void **retval)
 	else if (thread->flags & THREAD_DETACHED)
 		e = EINVAL;
 	else {
-		_sem_wait(&thread->donesem, 0, 0);
+		_sem_wait(&thread->donesem, 0);
 		if (retval)
 			*retval = thread->retval;
 		e = 0;
diff --git a/lib/librthread/rthread.h b/lib/librthread/rthread.h
index 21a8c165365..19af1f12eae 100644
--- a/lib/librthread/rthread.h
+++ b/lib/librthread/rthread.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rthread.h,v 1.24 2009/11/27 19:43:55 guenther Exp $ */
+/*	$OpenBSD: rthread.h,v 1.25 2009/11/27 19:45:54 guenther Exp $ */
 /*
  * Copyright (c) 2004,2005 Ted Unangst <tedu@openbsd.org>
  * All Rights Reserved.
@@ -141,8 +141,8 @@ extern _spinlock_lock_t _thread_lock;
 
 void	_spinlock(_spinlock_lock_t *);
 void	_spinunlock(_spinlock_lock_t *);
-int	_sem_wait(sem_t, int, int);
-int	_sem_waitl(sem_t, int, int);
+int	_sem_wait(sem_t, int);
+int	_sem_waitl(sem_t, int, clockid_t, const struct timespec *);
 int	_sem_post(sem_t);
 int	_sem_wakeup(sem_t);
 int	_sem_wakeall(sem_t);
@@ -166,7 +166,8 @@ int	_atomic_lock(register volatile _spinlock_lock_t *);
 /* syscalls */
 int getthrid(void);
 void threxit(pid_t *);
-int thrsleep(void *, int, void *);
+int thrsleep(const volatile void *, clockid_t, const struct timespec *,
+    volatile void *);
 int thrwakeup(void *, int n);
 int sched_yield(void);
 int thrsigdivert(sigset_t, siginfo_t *, const struct timespec *);
diff --git a/lib/librthread/rthread_file.c b/lib/librthread/rthread_file.c
index 673d7ad5d9f..95358514d0f 100644
--- a/lib/librthread/rthread_file.c
+++ b/lib/librthread/rthread_file.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rthread_file.c,v 1.1 2009/10/21 16:05:48 guenther Exp $	*/
+/*	$OpenBSD: rthread_file.c,v 1.2 2009/11/27 19:45:54 guenther Exp $	*/
 /*
  * Copyright (c) 1995 John Birrell <jb@cimlogic.com.au>.
  * All rights reserved.
@@ -206,7 +206,7 @@ void
 		 */
 		TAILQ_INSERT_TAIL(&p->lockers,self,waiting);
 		while (p->owner != self) {
-			thrsleep(self, 0, &hash_lock);
+			thrsleep(self, 0, NULL, &hash_lock);
 			_spinlock(&hash_lock);
 		}
 	}
diff --git a/lib/librthread/rthread_sync.c b/lib/librthread/rthread_sync.c
index a724c854d85..56b35d467dc 100644
--- a/lib/librthread/rthread_sync.c
+++ b/lib/librthread/rthread_sync.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: rthread_sync.c,v 1.21 2009/11/19 03:31:36 guenther Exp $ */
+/*	$OpenBSD: rthread_sync.c,v 1.22 2009/11/27 19:45:54 guenther Exp $ */
 /*
  * Copyright (c) 2004,2005 Ted Unangst <tedu@openbsd.org>
  * All Rights Reserved.
@@ -43,15 +43,16 @@ static _spinlock_lock_t static_init_lock = _SPINLOCK_UNLOCKED;
  * Internal implementation of semaphores
  */
 int
-_sem_wait(sem_t sem, int tryonly, int timo)
+_sem_wait(sem_t sem, int tryonly)
 {
 
 	_spinlock(&sem->lock);
-	return (_sem_waitl(sem, tryonly, timo));
+	return (_sem_waitl(sem, tryonly, 0, NULL));
 }
 
 int
-_sem_waitl(sem_t sem, int tryonly, int timo)
+_sem_waitl(sem_t sem, int tryonly, clockid_t clock_id,
+    const struct timespec *abstime)
 {
 	int do_sleep;
 
@@ -69,7 +70,7 @@ again:
 	}
 
 	if (do_sleep) {
-		if (thrsleep(sem, timo, &sem->lock) == -1 &&
+		if (thrsleep(sem, clock_id, abstime, &sem->lock) == -1 &&
 		    errno == EWOULDBLOCK)
 			return (0);
 		_spinlock(&sem->lock);
@@ -193,7 +194,7 @@ sem_wait(sem_t *semp)
 {
 	sem_t sem = *semp;
 
-	_sem_wait(sem, 0, 0);
+	_sem_wait(sem, 0);
 
 	return (0);
 }
@@ -204,7 +205,7 @@ sem_trywait(sem_t *semp)
 	sem_t sem = *semp;
 	int rv;
 
-	rv = _sem_wait(sem, 1, 0);
+	rv = _sem_wait(sem, 1);
 
 	if (!rv) {
 		errno = EAGAIN;
@@ -278,7 +279,7 @@ _rthread_mutex_lock(pthread_mutex_t *mutexp, int trywait)
 		if (mutex->type == PTHREAD_MUTEX_ERRORCHECK)
 			return (EDEADLK);
 	}
-	if (!_sem_wait((void *)&mutex->sem, trywait, 0))
+	if (!_sem_wait((void *)&mutex->sem, trywait))
 		return (EBUSY);
 	mutex->owner = thread;
 	mutex->count = 1;
@@ -391,29 +392,14 @@ pthread_cond_timedwait(pthread_cond_t *condp, pthread_mutex_t *mutexp,
 {
 	int error;
 	int rv;
-	int timo = 0;
-	struct timeval timenow;
-	struct timespec tmspec;
 
 	if (!*condp)
 		if ((error = pthread_cond_init(condp, NULL)))
 			return (error);
 
-	if (abstime && gettimeofday(&timenow, NULL) == 0) {
-		TIMEVAL_TO_TIMESPEC(&timenow, &tmspec);
-		if (timespeccmp(abstime, &tmspec, <)) {
-			pthread_mutex_unlock(mutexp);
-			error = pthread_mutex_lock(mutexp);
-			return (error ? error : ETIMEDOUT);
-		}
-		timespecsub(abstime, &tmspec, &tmspec);
-		timo = tmspec.tv_sec * 1000 + tmspec.tv_nsec / 1000000;
-	}
-
-
 	_spinlock(&(*condp)->sem.lock);
 	pthread_mutex_unlock(mutexp);
-	rv = _sem_waitl(&(*condp)->sem, 0, timo);
+	rv = _sem_waitl(&(*condp)->sem, 0, CLOCK_REALTIME, abstime);
 	error = pthread_mutex_lock(mutexp);
 
 	return (error ? error : rv ? 0 : ETIMEDOUT);
@@ -542,7 +528,7 @@ again:
 	_spinlock(&lock->lock);
 	if (lock->writer) {
 		_spinunlock(&lock->lock);
-		_sem_wait(&lock->sem, 0, 0);
+		_sem_wait(&lock->sem, 0);
 		goto again;
 	}
 	lock->readers++;
@@ -588,7 +574,7 @@ pthread_rwlock_wrlock(pthread_rwlock_t *lockp)
 	lock->writer++;
 	while (lock->readers) {
 		_spinunlock(&lock->lock);
-		_sem_wait(&lock->sem, 0, 0);
+		_sem_wait(&lock->sem, 0);
 		_spinlock(&lock->lock);
 	}
 	lock->readers = -pthread_self()->tid;
diff --git a/sys/kern/kern_synch.c b/sys/kern/kern_synch.c
index eb9d2f9dafa..565c4f35b95 100644
--- a/sys/kern/kern_synch.c
+++ b/sys/kern/kern_synch.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_synch.c,v 1.91 2009/06/04 04:26:54 beck Exp $	*/
+/*	$OpenBSD: kern_synch.c,v 1.92 2009/11/27 19:45:53 guenther Exp $	*/
 /*	$NetBSD: kern_synch.c,v 1.37 1996/04/22 01:38:37 christos Exp $	*/
 
 /*
@@ -407,24 +407,47 @@ sys_sched_yield(struct proc *p, void *v, register_t *retval)
 int
 sys_thrsleep(struct proc *p, void *v, register_t *revtal)
 {
-	struct sys_thrsleep_args *uap = v;
+	struct sys_thrsleep_args /* {
+		syscallarg(void *) ident;
+		syscallarg(clockid_t) clock_id;
+		syscallarg(struct timespec *) tp;
+		syscallarg(void *) lock;
+	} */ *uap = v;
 	long ident = (long)SCARG(uap, ident);
-	int timo = SCARG(uap, timeout);
 	_spinlock_lock_t *lock = SCARG(uap, lock);
-	_spinlock_lock_t unlocked = _SPINLOCK_UNLOCKED;
+	static _spinlock_lock_t unlocked = _SPINLOCK_UNLOCKED;
+	long long to_ticks = 0;
 	int error;
 
+	if (SCARG(uap, tp) != NULL) {
+		struct timespec now, ats;
+
+		if ((error = copyin(SCARG(uap, tp), &ats, sizeof(ats))) != 0 ||
+		    (error = clock_gettime(p, SCARG(uap, clock_id), &now)) != 0)
+			return (error);
+
+		if (timespeccmp(&ats, &now, <)) {
+			/* already passed: still do the unlock */
+			if (lock)
+				copyout(&unlocked, lock, sizeof(unlocked));
+			return (ETIMEDOUT);
+		}
+
+		timespecsub(&ats, &now, &ats);
+		to_ticks = (long long)hz * ats.tv_sec +
+		    ats.tv_nsec / (tick * 1000);
+		if (to_ticks > INT_MAX)
+			to_ticks = INT_MAX;
+		if (to_ticks == 0)
+			to_ticks = 1;
+	}
+
 	p->p_thrslpid = ident;
 
 	if (lock)
 		copyout(&unlocked, lock, sizeof(unlocked));
-	if (hz > 1000)
-		timo = timo * (hz / 1000);
-	else
-		timo = timo / (1000 / hz);
-	if (timo < 0)
-		timo = 0;
-	error = tsleep(&p->p_thrslpid, PUSER | PCATCH, "thrsleep", timo);
+	error = tsleep(&p->p_thrslpid, PUSER | PCATCH, "thrsleep",
+	    (int)to_ticks);
 
 	if (error == ERESTART)
 		error = EINTR;
@@ -436,7 +459,10 @@ sys_thrsleep(struct proc *p, void *v, register_t *revtal)
 int
 sys_thrwakeup(struct proc *p, void *v, register_t *retval)
 {
-	struct sys_thrwakeup_args *uap = v;
+	struct sys_thrwakeup_args /* {
+		syscallarg(void *) ident;
+		syscallarg(int) n;
+	} */ *uap = v;
 	long ident = (long)SCARG(uap, ident);
 	int n = SCARG(uap, n);
 	struct proc *q;
diff --git a/sys/kern/kern_time.c b/sys/kern/kern_time.c
index 82a805cffd8..6cb2aa8a320 100644
--- a/sys/kern/kern_time.c
+++ b/sys/kern/kern_time.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kern_time.c,v 1.67 2009/10/16 19:29:41 martynas Exp $	*/
+/*	$OpenBSD: kern_time.c,v 1.68 2009/11/27 19:45:53 guenther Exp $	*/
 /*	$NetBSD: kern_time.c,v 1.20 1996/02/18 11:57:06 fvdl Exp $	*/
 
 /*
@@ -156,32 +156,39 @@ settime(struct timespec *ts)
 }
 #endif
 
-/* ARGSUSED */
 int
-sys_clock_gettime(struct proc *p, void *v, register_t *retval)
+clock_gettime(struct proc *p, clockid_t clock_id, struct timespec *tp)
 {
-	struct sys_clock_gettime_args /* {
-		syscallarg(clockid_t) clock_id;
-		syscallarg(struct timespec *) tp;
-	} */ *uap = v;
-	clockid_t clock_id;
-	struct timespec ats;
-
-	clock_id = SCARG(uap, clock_id);
 	switch (clock_id) {
 	case CLOCK_REALTIME:
-		nanotime(&ats);
+		nanotime(tp);
 		break;
 	case CLOCK_MONOTONIC:
-		nanouptime(&ats);
+		nanouptime(tp);
 		break;
 	case CLOCK_PROF:
-		ats.tv_sec = p->p_rtime.tv_sec;
-		ats.tv_nsec = p->p_rtime.tv_usec * 1000;
+		tp->tv_sec = p->p_rtime.tv_sec;
+		tp->tv_nsec = p->p_rtime.tv_usec * 1000;
 		break;
 	default:
 		return (EINVAL);
 	}
+	return (0);
+}
+
+/* ARGSUSED */
+int
+sys_clock_gettime(struct proc *p, void *v, register_t *retval)
+{
+	struct sys_clock_gettime_args /* {
+		syscallarg(clockid_t) clock_id;
+		syscallarg(struct timespec *) tp;
+	} */ *uap = v;
+	struct timespec ats;
+	int error;
+
+	if ((error = clock_gettime(p, SCARG(uap, clock_id), &ats)) != 0)
+		return (error);
 
 	return copyout(&ats, SCARG(uap, tp), sizeof(ats));
 }
@@ -688,6 +695,18 @@ realitexpire(void *arg)
 }
 
 /*
+ * Check that a timespec value is legit
+ */
+int
+timespecfix(struct timespec *ts)
+{
+	if (ts->tv_sec < 0 || ts->tv_sec > 100000000 ||
+	    ts->tv_nsec < 0 || ts->tv_nsec >= 1000000000)
+		return (EINVAL);
+	return (0);
+}
+
+/*
  * Check that a proposed value to load into the .it_value or
  * .it_interval part of an interval timer is acceptable.
  */
diff --git a/sys/kern/syscalls.master b/sys/kern/syscalls.master
index 7692a85edce..fba9aad506c 100644
--- a/sys/kern/syscalls.master
+++ b/sys/kern/syscalls.master
@@ -1,4 +1,4 @@
-;	$OpenBSD: syscalls.master,v 1.95 2009/11/27 19:43:55 guenther Exp $
+;	$OpenBSD: syscalls.master,v 1.96 2009/11/27 19:45:53 guenther Exp $
 ;	$NetBSD: syscalls.master,v 1.32 1996/04/23 10:24:21 mycroft Exp $
 
 ;	@(#)syscalls.master	8.2 (Berkeley) 1/13/94
@@ -597,7 +597,8 @@
 298	STD		{ int sys_sched_yield(void); }
 #ifdef RTHREADS
 299	STD		{ pid_t sys_getthrid(void); }
-300	STD		{ int sys_thrsleep(void *ident, int timeout, void *lock); }
+300	STD		{ int sys_thrsleep(void *ident, clockid_t clock_id, \
+			    const struct timespec *tp, void *lock); }
 301	STD		{ int sys_thrwakeup(void *ident, int n); }
 302	STD		{ void sys_threxit(pid_t *notdead); }
 303	STD		{ int sys_thrsigdivert(sigset_t sigmask, \
diff --git a/sys/sys/syscall.h b/sys/sys/syscall.h
index 8dc92f61d30..2084589b4da 100644
--- a/sys/sys/syscall.h
+++ b/sys/sys/syscall.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: syscall.h,v 1.107 2009/08/02 16:28:40 beck Exp $	*/
+/*	$OpenBSD: syscall.h,v 1.108 2009/11/27 19:45:53 guenther Exp $	*/
 
 /*
  * System call numbers.
@@ -674,7 +674,7 @@
 /* syscall: "thrwakeup" ret: "int" args: "void *" "int" */
 #define	SYS_thrwakeup	301
 
-/* syscall: "threxit" ret: "void" args: "int" */
+/* syscall: "threxit" ret: "void" args: "pid_t *" */
 #define	SYS_threxit	302
 
 /* syscall: "thrsigdivert" ret: "int" args: "sigset_t" */
diff --git a/sys/sys/syscallargs.h b/sys/sys/syscallargs.h
index ae8104b81b9..c5a252a01f6 100644
--- a/sys/sys/syscallargs.h
+++ b/sys/sys/syscallargs.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: syscallargs.h,v 1.109 2009/08/02 16:28:40 beck Exp $	*/
+/*	$OpenBSD: syscallargs.h,v 1.110 2009/11/27 19:45:53 guenther Exp $	*/
 
 /*
  * System call argument lists.
@@ -1221,7 +1221,7 @@ struct sys_thrwakeup_args {
 };
 
 struct sys_threxit_args {
-	syscallarg(int) rval;
+	syscallarg(pid_t *) notdead;
 };
 
 struct sys_thrsigdivert_args {
diff --git a/sys/sys/time.h b/sys/sys/time.h
index 2104de51506..85a3905c888 100644
--- a/sys/sys/time.h
+++ b/sys/sys/time.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: time.h,v 1.25 2007/05/09 17:42:19 deraadt Exp $	*/
+/*	$OpenBSD: time.h,v 1.26 2009/11/27 19:45:54 guenther Exp $	*/
 /*	$NetBSD: time.h,v 1.18 1996/04/23 10:29:33 mycroft Exp $	*/
 
 /*
@@ -304,6 +304,10 @@ void	microuptime(struct timeval *);
 void	getnanouptime(struct timespec *);
 void	getmicrouptime(struct timeval *);
 
+struct proc;
+int	clock_gettime(struct proc *, clockid_t, struct timespec *);
+
+int	timespecfix(struct timespec *);
 int	itimerfix(struct timeval *);
 int	itimerdecr(struct itimerval *itp, int usec);
 int	settime(struct timespec *);