From 53759dd71238c5c613ab3f0ca261a5180fa3fad7 Mon Sep 17 00:00:00 2001 From: EunBong Song Date: Thu, 13 Apr 2017 13:43:32 +0900 Subject: [PATCH] pthread/mutex: fix an assertion noted by Jussi Kivilinna All credits should go to Gregory Nutt who wrote the original commit. Change-Id: I3ba2c8a4bccc8c76047d6b22a36662c79f8ac33d Signed-off-by: Gregory Nutt [Song: backported eb344d72 from NuttX] Signed-off-by: EunBong Song --- os/kernel/pthread/pthread.h | 2 + os/kernel/pthread/pthread_mutextrylock.c | 103 +++++++++++++++---------------- 2 files changed, 53 insertions(+), 52 deletions(-) diff --git a/os/kernel/pthread/pthread.h b/os/kernel/pthread/pthread.h index ca5750b..bc0eda1 100644 --- a/os/kernel/pthread/pthread.h +++ b/os/kernel/pthread/pthread.h @@ -126,10 +126,12 @@ int pthread_givesemaphore(sem_t *sem); #ifndef CONFIG_PTHREAD_MUTEX_UNSAFE int pthread_mutex_take(FAR struct pthread_mutex_s *mutex, bool intr); +int pthread_mutex_trytake(FAR struct pthread_mutex_s *mutex); int pthread_mutex_give(FAR struct pthread_mutex_s *mutex); void pthread_mutex_inconsistent(FAR struct pthread_tcb_s *tcb); #else #define pthread_mutex_take(m,i) pthread_takesemaphore(&(m)->sem,(i)) +#define pthread_mutex_trytake(m) sem_trywait(&(m)->sem) #define pthread_mutex_give(m) pthread_givesemaphore(&(m)->sem) #endif diff --git a/os/kernel/pthread/pthread_mutextrylock.c b/os/kernel/pthread/pthread_mutextrylock.c index f6f6560..b71e796 100644 --- a/os/kernel/pthread/pthread_mutextrylock.c +++ b/os/kernel/pthread/pthread_mutextrylock.c @@ -138,7 +138,7 @@ int pthread_mutex_trylock(FAR pthread_mutex_t *mutex) /* Try to get the semaphore. */ - status = sem_trywait((FAR sem_t *)&mutex->sem); + status = pthread_mutex_trytake(mutex); if (status == OK) { /* If we successfully obtained the semaphore, then indicate * that we own it. @@ -152,82 +152,81 @@ int pthread_mutex_trylock(FAR pthread_mutex_t *mutex) ret = OK; } - /* sem_trywait failed */ + /* pthread_mutex_trytake failed. Did it fail because the semaphore + * was not avaialable? + */ - else { - /* Did it fail because the semaphore was not available? */ - int errcode = get_errno(); - if (errcode == EAGAIN) { + else if (status == EAGAIN) { #ifdef CONFIG_PTHREAD_MUTEX_TYPES - /* Check if recursive mutex was locked by the calling thread. */ - - if (mutex->type == PTHREAD_MUTEX_RECURSIVE && mutex->pid == mypid) { - /* Increment the number of locks held and return successfully. */ - - if (mutex->nlocks < INT16_MAX) { - mutex->nlocks++; - ret = OK; - } else { - ret = EOVERFLOW; - } - } else + /* Check if recursive mutex was locked by the calling thread. */ + + if (mutex->type == PTHREAD_MUTEX_RECURSIVE && mutex->pid == mypid) { + + /* Increment the number of locks held and return successfully. */ + + if (mutex->nlocks < INT16_MAX) { + mutex->nlocks++; + ret = OK; + } else { + ret = EOVERFLOW; + } + } else #endif #ifndef CONFIG_PTHREAD_MUTEX_UNSAFE - /* The calling thread does not hold the semaphore. The correct - * behavior for the 'robust' mutex is to verify that the holder of - * the mutex is still valid. This is protection from the case - * where the holder of the mutex has exitted without unlocking it. - */ + /* The calling thread does not hold the semaphore. The correct + * behavior for the 'robust' mutex is to verify that the holder of + * the mutex is still valid. This is protection from the case + * where the holder of the mutex has exitted without unlocking it. + */ #ifdef CONFIG_PTHREAD_MUTEX_BOTH #ifdef CONFIG_PTHREAD_MUTEX_TYPES - /* Check if this NORMAL mutex is robust */ + /* Check if this NORMAL mutex is robust */ - if (mutex->pid > 0 && - ((mutex->flags & _PTHREAD_MFLAGS_ROBUST) != 0 || - mutex->type != PTHREAD_MUTEX_NORMAL) && - sched_gettcb(mutex->pid) == NULL) + if (mutex->pid > 0 && + ((mutex->flags & _PTHREAD_MFLAGS_ROBUST) != 0 || + mutex->type != PTHREAD_MUTEX_NORMAL) && + sched_gettcb(mutex->pid) == NULL) #else /* CONFIG_PTHREAD_MUTEX_TYPES */ - /* Check if this NORMAL mutex is robust */ + /* Check if this NORMAL mutex is robust */ - if (mutex->pid > 0 && - (mutex->flags & _PTHREAD_MFLAGS_ROBUST) != 0 && - sched_gettcb(mutex->pid) == NULL) + if (mutex->pid > 0 && + (mutex->flags & _PTHREAD_MFLAGS_ROBUST) != 0 && + sched_gettcb(mutex->pid) == NULL) #endif /* CONFIG_PTHREAD_MUTEX_TYPES */ #else /* CONFIG_PTHREAD_MUTEX_ROBUST */ - /* This mutex is always robust, whatever type it is. */ - if (mutex->pid > 0 && sched_gettcb(mutex->pid) == NULL) + /* This mutex is always robust, whatever type it is. */ + if (mutex->pid > 0 && sched_gettcb(mutex->pid) == NULL) #endif - { - DEBUGASSERT(mutex->pid != 0); /* < 0: available, >0 owned, ==0 error */ - DEBUGASSERT((mutex->flags & _PTHREAD_MFLAGS_INCONSISTENT) != 0); - - /* A thread holds the mutex, but there is no such thread. - * POSIX requires that the 'robust' mutex return EOWNERDEAD - * in this case. It is then the caller's responsibility to - * call pthread_mutx_consistent() fo fix the mutex. - */ + { + DEBUGASSERT(mutex->pid != 0); /* < 0: available, >0 owned, ==0 error */ + DEBUGASSERT((mutex->flags & _PTHREAD_MFLAGS_INCONSISTENT) != 0); + + /* A thread holds the mutex, but there is no such thread. + * POSIX requires that the 'robust' mutex return EOWNERDEAD + * in this case. It is then the caller's responsibility to + * call pthread_mutx_consistent() fo fix the mutex. + */ - mutex->flags |= _PTHREAD_MFLAGS_INCONSISTENT; - ret = EOWNERDEAD; - } + mutex->flags |= _PTHREAD_MFLAGS_INCONSISTENT; + ret = EOWNERDEAD; + } /* The mutex is locked by another, active thread */ - else + else #endif /* CONFIG_PTHREAD_MUTEX_UNSAFE */ - { + { ret = EBUSY; - } + } } - + /* Some other, unhandled error occurred */ else { - ret = errcode; + ret = status; } - } sched_unlock(); } -- 2.7.4