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 <gnutt@nuttx.org>
[Song: backported eb344d72 from NuttX]
Signed-off-by: EunBong Song <eunb.song@samsung.com>

diff --git a/os/kernel/pthread/pthread.h b/os/kernel/pthread/pthread.h
index ca5750b..bc0eda1 100644 (file)
--- 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 (file)
--- 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();
        }