Hi, After discussing further with mpi@ and guenther@, we decided to first fix the existing semaphore implementation with regards to SA_RESTART and POSIX compliant returns in the case where we deal with restartable signals.
Currently we return EINTR everywhere which is mostly incorrect as the user can not know if she needs to recall the syscall or not. Philip suggested we use ECANCELED for that which is what my diff does. Regression tests pass and so does the posixsuite. Timespec validation bits are needed to pass the later. Branch available at https://github.com/bulibuta/openbsd-src/tree/oldsem_ecanceled Thoughts? OKs? Paul diff --git lib/libc/thread/rthread_sync.c lib/libc/thread/rthread_sync.c index 91ce55cbcf9..bb9a26803fa 100644 --- lib/libc/thread/rthread_sync.c +++ lib/libc/thread/rthread_sync.c @@ -375,7 +375,8 @@ pthread_cond_timedwait(pthread_cond_t *condp, pthread_mutex_t *mutexp, * cancellation) then we should just go back to * sleep without changing state (timeouts, etc). */ - if (error == EINTR && (tib->tib_canceled == 0 || + if ((error == EINTR || error == ECANCELED) && + (tib->tib_canceled == 0 || (tib->tib_cantcancel & CANCEL_DISABLED))) { _spinlock(&mutex->lock); continue; @@ -514,7 +515,8 @@ pthread_cond_wait(pthread_cond_t *condp, pthread_mutex_t *mutexp) * cancellation) then we should just go back to * sleep without changing state (timeouts, etc). */ - if (error == EINTR && (tib->tib_canceled == 0 || + if ((error == EINTR || error == ECANCELED) && + (tib->tib_canceled == 0 || (tib->tib_cantcancel & CANCEL_DISABLED))) { _spinlock(&mutex->lock); continue; diff --git lib/librthread/rthread_rwlock.c lib/librthread/rthread_rwlock.c index a75e88c52e4..6fccd2fe1bd 100644 --- lib/librthread/rthread_rwlock.c +++ lib/librthread/rthread_rwlock.c @@ -143,8 +143,8 @@ int pthread_rwlock_timedrdlock(pthread_rwlock_t *lockp, const struct timespec *abstime) { - if (abstime == NULL || abstime->tv_sec < 0 || abstime->tv_nsec < 0 || - abstime->tv_nsec > 1000000000) + if (abstime == NULL || abstime->tv_nsec < 0 || + abstime->tv_nsec >= 1000000000) return (EINVAL); return (_rthread_rwlock_rdlock(lockp, abstime, 0)); } @@ -210,8 +210,8 @@ int pthread_rwlock_timedwrlock(pthread_rwlock_t *lockp, const struct timespec *abstime) { - if (abstime == NULL || abstime->tv_sec < 0 || abstime->tv_nsec < 0 || - abstime->tv_nsec > 1000000000) + if (abstime == NULL || abstime->tv_nsec < 0 || + abstime->tv_nsec >= 1000000000) return (EINVAL); return (_rthread_rwlock_wrlock(lockp, abstime, 0)); } diff --git lib/librthread/rthread_sem.c lib/librthread/rthread_sem.c index 196fb8ec357..d4c133aa4e0 100644 --- lib/librthread/rthread_sem.c +++ lib/librthread/rthread_sem.c @@ -75,7 +75,7 @@ _sem_wait(sem_t sem, int tryonly, const struct timespec *abstime, &sem->lock, delayed_cancel); _spinlock(&sem->lock); /* ignore interruptions other than cancelation */ - if (r == EINTR && (delayed_cancel == NULL || + if (r == ECANCELED && (delayed_cancel == NULL || *delayed_cancel == 0)) r = 0; } while (r == 0 && sem->value == 0); @@ -268,15 +268,16 @@ sem_timedwait(sem_t *semp, const struct timespec *abstime) int r; PREP_CANCEL_POINT(tib); - if (!_threads_ready) - _rthread_init(); - self = tib->tib_thread; - - if (!semp || !(sem = *semp)) { + if (!semp || !(sem = *semp) || abstime == NULL || + abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000) { errno = EINVAL; return (-1); } + if (!_threads_ready) + _rthread_init(); + self = tib->tib_thread; + ENTER_DELAYED_CANCEL_POINT(tib, self); r = _sem_wait(sem, 0, abstime, &self->delayed_cancel); LEAVE_CANCEL_POINT_INNER(tib, r); diff --git sys/kern/kern_synch.c sys/kern/kern_synch.c index ea28288d375..7a920b27c8b 100644 --- sys/kern/kern_synch.c +++ sys/kern/kern_synch.c @@ -592,7 +592,7 @@ out: p->p_thrslpid = 0; if (error == ERESTART) - error = EINTR; + error = ECANCELED; return (error); @@ -614,13 +614,17 @@ sys___thrsleep(struct proc *p, void *v, register_t *retval) if (SCARG(uap, tp) != NULL) { if ((error = copyin(SCARG(uap, tp), &ts, sizeof(ts)))) { *retval = error; - return (0); + return 0; + } + if (ts.tv_nsec < 0 || ts.tv_nsec >= 1000000000) { + *retval = EINVAL; + return 0; } SCARG(uap, tp) = &ts; } *retval = thrsleep(p, uap); - return (0); + return 0; } int
