From 2c37d369abae9a963a9ee2e3b3f673c5956a8da1 Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Sun, 26 Mar 2017 11:22:17 -0600 Subject: [PATCH] pthread: Fix return value of pthread_give/takesemaphore(). Add option to pthread_takesemaphore to ignore EINTR or not. --- sched/pthread/pthread.h | 2 +- sched/pthread/pthread_completejoin.c | 4 +-- sched/pthread/pthread_condtimedwait.c | 16 +++++------ sched/pthread/pthread_condwait.c | 21 +++++++++++++-- sched/pthread/pthread_create.c | 4 +-- sched/pthread/pthread_detach.c | 2 +- sched/pthread/pthread_initialize.c | 39 ++++++++++++++------------- sched/pthread/pthread_join.c | 6 ++--- sched/pthread/pthread_mutexlock.c | 10 ++++++- 9 files changed, 64 insertions(+), 40 deletions(-) diff --git a/sched/pthread/pthread.h b/sched/pthread/pthread.h index be7a20de2e..858e80ed07 100644 --- a/sched/pthread/pthread.h +++ b/sched/pthread/pthread.h @@ -108,7 +108,7 @@ FAR struct join_s *pthread_findjoininfo(FAR struct task_group_s *group, pid_t pid); void pthread_release(FAR struct task_group_s *group); int pthread_givesemaphore(sem_t *sem); -int pthread_takesemaphore(sem_t *sem); +int pthread_takesemaphore(sem_t *sem, bool intr); #ifdef CONFIG_MUTEX_TYPES int pthread_mutexattr_verifytype(int type); diff --git a/sched/pthread/pthread_completejoin.c b/sched/pthread/pthread_completejoin.c index b516c9defd..6053f6c1da 100644 --- a/sched/pthread/pthread_completejoin.c +++ b/sched/pthread/pthread_completejoin.c @@ -99,7 +99,7 @@ static bool pthread_notifywaiters(FAR struct join_s *pjoin) * value. */ - (void)pthread_takesemaphore(&pjoin->data_sem); + (void)pthread_takesemaphore(&pjoin->data_sem, false); return true; } @@ -210,7 +210,7 @@ int pthread_completejoin(pid_t pid, FAR void *exit_value) /* First, find thread's structure in the private data set. */ - (void)pthread_takesemaphore(&group->tg_joinsem); + (void)pthread_takesemaphore(&group->tg_joinsem, false); pjoin = pthread_findjoininfo(group, pid); if (!pjoin) { diff --git a/sched/pthread/pthread_condtimedwait.c b/sched/pthread/pthread_condtimedwait.c index 9027c305ef..960f46483c 100644 --- a/sched/pthread/pthread_condtimedwait.c +++ b/sched/pthread/pthread_condtimedwait.c @@ -156,12 +156,10 @@ static void pthread_condtimedout(int argc, uint32_t pid, uint32_t signo) * abstime - wait until this absolute time * * Return Value: - * OK (0) on success; ERROR (-1) on failure with errno - * set appropriately. + * OK (0) on success; A non-zero errno value is returned on failure. * * Assumptions: - * Timing is of resolution 1 msec, with +/-1 millisecond - * accuracy. + * Timing is of resolution 1 msec, with +/-1 millisecond accuracy. * ****************************************************************************/ @@ -236,7 +234,7 @@ int pthread_cond_timedwait(FAR pthread_cond_t *cond, FAR pthread_mutex_t *mutex, if (ret) { /* Restore interrupts (pre-emption will be enabled when - * we fall through the if/then/else + * we fall through the if/then/else) */ leave_critical_section(flags); @@ -263,7 +261,7 @@ int pthread_cond_timedwait(FAR pthread_cond_t *cond, FAR pthread_mutex_t *mutex, mutex->pid = -1; ret = pthread_givesemaphore((FAR sem_t *)&mutex->sem); - if (ret) + if (ret != 0) { /* Restore interrupts (pre-emption will be enabled when * we fall through the if/then/else) @@ -318,12 +316,12 @@ int pthread_cond_timedwait(FAR pthread_cond_t *cond, FAR pthread_mutex_t *mutex, /* Reacquire the mutex (retaining the ret). */ sinfo("Re-locking...\n"); - status = pthread_takesemaphore((FAR sem_t *)&mutex->sem); - if (!status) + status = pthread_takesemaphore((FAR sem_t *)&mutex->sem, false); + if (status == OK) { mutex->pid = mypid; } - else if (!ret) + else if (ret == 0) { ret = status; } diff --git a/sched/pthread/pthread_condwait.c b/sched/pthread/pthread_condwait.c index e7c6a9af25..a434f3032c 100644 --- a/sched/pthread/pthread_condwait.c +++ b/sched/pthread/pthread_condwait.c @@ -71,6 +71,7 @@ int pthread_cond_wait(FAR pthread_cond_t *cond, FAR pthread_mutex_t *mutex) { + int status; int ret; sinfo("cond=0x%p mutex=0x%p\n", cond, mutex); @@ -104,7 +105,14 @@ int pthread_cond_wait(FAR pthread_cond_t *cond, FAR pthread_mutex_t *mutex) /* Take the semaphore */ - ret |= pthread_takesemaphore((FAR sem_t *)&cond->sem); + status = pthread_takesemaphore((FAR sem_t *)&cond->sem, false); + if (ret == OK) + { + /* Report the first failure that occurs */ + + ret = status; + } + sched_unlock(); /* Reacquire the mutex. @@ -114,7 +122,16 @@ int pthread_cond_wait(FAR pthread_cond_t *cond, FAR pthread_mutex_t *mutex) */ sinfo("Reacquire mutex...\n"); - ret |= pthread_takesemaphore((FAR sem_t *)&mutex->sem); + status = pthread_takesemaphore((FAR sem_t *)&mutex->sem, false); + if (ret == OK) + { + /* Report the first failure that occurs */ + + ret = status; + } + + /* Was all of the above successful? */ + if (ret == OK) { mutex->pid = getpid(); diff --git a/sched/pthread/pthread_create.c b/sched/pthread/pthread_create.c index 957936cd82..4fbf646424 100644 --- a/sched/pthread/pthread_create.c +++ b/sched/pthread/pthread_create.c @@ -177,7 +177,7 @@ static void pthread_start(void) /* Sucessfully spawned, add the pjoin to our data set. */ - (void)pthread_takesemaphore(&group->tg_joinsem); + (void)pthread_takesemaphore(&group->tg_joinsem, false); pthread_addjoininfo(group, pjoin); (void)pthread_givesemaphore(&group->tg_joinsem); @@ -555,7 +555,7 @@ int pthread_create(FAR pthread_t *thread, FAR const pthread_attr_t *attr, * its join structure. */ - (void)pthread_takesemaphore(&pjoin->data_sem); + (void)pthread_takesemaphore(&pjoin->data_sem, false); /* Return the thread information to the caller */ diff --git a/sched/pthread/pthread_detach.c b/sched/pthread/pthread_detach.c index 0d165e2d23..9e3d900d7e 100644 --- a/sched/pthread/pthread_detach.c +++ b/sched/pthread/pthread_detach.c @@ -87,7 +87,7 @@ int pthread_detach(pthread_t thread) /* Find the entry associated with this pthread. */ - (void)pthread_takesemaphore(&group->tg_joinsem); + (void)pthread_takesemaphore(&group->tg_joinsem, false); pjoin = pthread_findjoininfo(group, (pid_t)thread); if (!pjoin) { diff --git a/sched/pthread/pthread_initialize.c b/sched/pthread/pthread_initialize.c index 0ced8d6181..e5965d8b60 100644 --- a/sched/pthread/pthread_initialize.c +++ b/sched/pthread/pthread_initialize.c @@ -40,7 +40,9 @@ #include #include +#include #include +#include #include #include "pthread/pthread.h" @@ -63,8 +65,6 @@ * Return Value: * None * - * Assumptions: - * ****************************************************************************/ void pthread_initialize(void) @@ -78,44 +78,45 @@ void pthread_initialize(void) * Support managed access to the private data sets. * * Parameters: - * None + * sem - The semaphore to lock or unlock + * intr - false: ignore EINTR errors when locking; true tread EINTR as + * other errors by returning the errno value * * Return Value: - * 0 on success or an ERROR on failure with errno value set to EINVAL. - * Note that the errno EINTR is never returned by this function. - * - * Assumptions: + * 0 on success or an errno value on failure. * ****************************************************************************/ -int pthread_takesemaphore(sem_t *sem) +int pthread_takesemaphore(sem_t *sem, bool intr) { /* Verify input parameters */ - if (sem) + DEBUGASSERT(sem != NULL); + if (sem != NULL) { /* Take the semaphore */ while (sem_wait(sem) != OK) { + int errcode = get_errno(); + /* Handle the special case where the semaphore wait was * awakened by the receipt of a signal. */ - if (get_errno() != EINTR) + if (intr || errcode != EINTR) { - set_errno(EINVAL); - return ERROR; + return errcode; } } + return OK; } else { /* NULL semaphore pointer! */ - set_errno(EINVAL); - return ERROR; + return EINVAL; } } @@ -123,7 +124,9 @@ int pthread_givesemaphore(sem_t *sem) { /* Verify input parameters */ - if (sem) + + DEBUGASSERT(sem != NULL); + if (sem != NULL) { /* Give the semaphore */ @@ -135,16 +138,14 @@ int pthread_givesemaphore(sem_t *sem) { /* sem_post() reported an error */ - set_errno(EINVAL); - return ERROR; + return get_errno(); } } else { /* NULL semaphore pointer! */ - set_errno(EINVAL); - return ERROR; + return EINVAL; } } diff --git a/sched/pthread/pthread_join.c b/sched/pthread/pthread_join.c index 01c1c713ea..6d2b48c81a 100644 --- a/sched/pthread/pthread_join.c +++ b/sched/pthread/pthread_join.c @@ -114,7 +114,7 @@ int pthread_join(pthread_t thread, FAR pthread_addr_t *pexit_value) * because it will also attempt to get this semaphore. */ - (void)pthread_takesemaphore(&group->tg_joinsem); + (void)pthread_takesemaphore(&group->tg_joinsem, false); /* Find the join information associated with this thread. * This can fail for one of three reasons: (1) There is no @@ -197,7 +197,7 @@ int pthread_join(pthread_t thread, FAR pthread_addr_t *pexit_value) * pthread to exit. */ - (void)pthread_takesemaphore(&pjoin->exit_sem); + (void)pthread_takesemaphore(&pjoin->exit_sem, false); /* The thread has exited! Get the thread exit value */ @@ -217,7 +217,7 @@ int pthread_join(pthread_t thread, FAR pthread_addr_t *pexit_value) * pthread_destroyjoin is called. */ - (void)pthread_takesemaphore(&group->tg_joinsem); + (void)pthread_takesemaphore(&group->tg_joinsem, false); } /* Pre-emption is okay now. The logic still cannot be re-entered diff --git a/sched/pthread/pthread_mutexlock.c b/sched/pthread/pthread_mutexlock.c index e14a7de17e..657bede370 100644 --- a/sched/pthread/pthread_mutexlock.c +++ b/sched/pthread/pthread_mutexlock.c @@ -176,7 +176,7 @@ int pthread_mutex_lock(FAR pthread_mutex_t *mutex) { /* Take the underlying semaphore, waiting if necessary */ - ret = pthread_takesemaphore((FAR sem_t *)&mutex->sem); + ret = pthread_takesemaphore((FAR sem_t *)&mutex->sem, true); /* If we successfully obtained the semaphore, then indicate * that we own it. @@ -189,6 +189,14 @@ int pthread_mutex_lock(FAR pthread_mutex_t *mutex) mutex->nlocks = 1; #endif } + + /* Check if we were awakened by a signal. This might happen if the + * tasking holding the mutex just exitted. + */ + + else if (ret == EINTR) + { + } } sched_unlock();