Fix nasa#673 nasa#677, improve global lock on POSIX

Removes the signal mask updates from the POSIX global lock (not needed).
Adds a condition variable to the structure, which can be used to
directly wake up a waiting task rather than requiring that task to poll
the global state.

src/os/posix/src/os-impl-common.c

@@ -165,3 +165,4 @@ void OS_Posix_CompAbsDelayTime(uint32 msecs, struct timespec *tm)
         tm->tv_sec++;
     }
 } /* end OS_CompAbsDelayTime */
+

src/os/posix/src/os-impl-idmap.c

@@ -38,7 +38,7 @@
 typedef struct
 {
     pthread_mutex_t mutex;
-    sigset_t        sigmask;
+    pthread_cond_t  cond;
 } POSIX_GlobalLock_t;
 
 static POSIX_GlobalLock_t OS_global_task_table_mut;
@@ -86,29 +86,29 @@ enum
 int32 OS_Lock_Global_Impl(osal_objtype_t idtype)
 {
     POSIX_GlobalLock_t *mut;
-    sigset_t            previous;
+    int ret;
 
-    mut = MUTEX_TABLE[idtype];
-
-    if (mut == NULL)
+    if (idtype < MUTEX_TABLE_SIZE)
     {
-        return OS_ERROR;
+        mut = MUTEX_TABLE[idtype];
     }
-
-    if (pthread_sigmask(SIG_SETMASK, &POSIX_GlobalVars.MaximumSigMask, &previous) != 0)
+    else
     {
-        return OS_ERROR;
+        mut = NULL;
     }
-
-    if (pthread_mutex_lock(&mut->mutex) != 0)
+    
+    if (mut != NULL)
     {
-        return OS_ERROR;
+        ret = pthread_mutex_lock(&mut->mutex);
+        if (ret != 0)
+        {
+            OS_DEBUG("pthread_mutex_lock(&mut->mutex): %s", strerror(ret));
+            return OS_ERROR;
+        }
     }
 
-    /* Only set values inside the GlobalLock _after_ it is locked */
-    mut->sigmask = previous;
-
     return OS_SUCCESS;
+
 } /* end OS_Lock_Global_Impl */
 
 /*----------------------------------------------------------------
@@ -122,7 +122,7 @@ int32 OS_Lock_Global_Impl(osal_objtype_t idtype)
 int32 OS_Unlock_Global_Impl(osal_objtype_t idtype)
 {
     POSIX_GlobalLock_t *mut;
-    sigset_t            previous;
+    int ret;
 
     if (idtype < MUTEX_TABLE_SIZE)
     {
@@ -133,23 +133,66 @@ int32 OS_Unlock_Global_Impl(osal_objtype_t idtype)
         mut = NULL;
     }
 
-    if (mut == NULL)
+    if (mut != NULL)
     {
-        return OS_ERROR;
+        /* Notify any waiting threads that the state _may_ have changed */
+        ret = pthread_cond_broadcast(&mut->cond);
+        if (ret != 0)
+        {
+            OS_DEBUG("pthread_cond_broadcast(&mut->cond): %s", strerror(ret));
+            /* unexpected but keep going (not critical) */
+        }
+
+        ret = pthread_mutex_unlock(&mut->mutex);
+        if (ret != 0)
+        {
+            OS_DEBUG("pthread_mutex_unlock(&mut->mutex): %s", strerror(ret));
+            return OS_ERROR;
+        }
     }
 
-    /* Only get values inside the GlobalLock _before_ it is unlocked */
-    previous = mut->sigmask;
+    return OS_SUCCESS;
+} /* end OS_Unlock_Global_Impl */
+
+/*----------------------------------------------------------------
+ *
+ *  Function: OS_WaitForStateChange_Impl
+ *
+ *  Purpose: Implemented per internal OSAL API
+ *           See prototype for argument/return detail
+ *
+ *-----------------------------------------------------------------*/
+void OS_WaitForStateChange_Impl(osal_objtype_t idtype, uint32 attempts)
+{
+    POSIX_GlobalLock_t *impl;
+    struct timespec ts;
+
+    impl = MUTEX_TABLE[idtype];
 
-    if (pthread_mutex_unlock(&mut->mutex) != 0)
+    if (impl != NULL)
     {
-        return OS_ERROR;
-    }
+        clock_gettime(CLOCK_REALTIME, &ts);
+
+        if (attempts <= 10)
+        {
+            /* Wait an increasing amount of time, starting at 10ms */
+            ts.tv_nsec += attempts * attempts * 10000000;
+            if (ts.tv_nsec >= 1000000000)
+            {
+                ts.tv_nsec -= 1000000000;
+                ++ts.tv_sec;
+            }
+        }
+        else
+        {
+            /* wait 1 second (max for polling) */
+            ++ts.tv_sec;
+        }
 
-    pthread_sigmask(SIG_SETMASK, &previous, NULL);
+        pthread_cond_timedwait(&impl->cond, &impl->mutex, &ts);
+    }
+}
 
-    return OS_SUCCESS;
-} /* end OS_Unlock_Global_Impl */
 
 /*---------------------------------------------------------------------------------------
    Name: OS_Posix_TableMutex_Init
@@ -163,6 +206,7 @@ int32 OS_Posix_TableMutex_Init(osal_objtype_t idtype)
     int                 ret;
     int32               return_code = OS_SUCCESS;
     pthread_mutexattr_t mutex_attr;
+    POSIX_GlobalLock_t *impl;
 
     do
     {
@@ -171,14 +215,16 @@ int32 OS_Posix_TableMutex_Init(osal_objtype_t idtype)
             break;
         }
 
+        impl = MUTEX_TABLE[idtype];
+
         /* Initialize the table mutex for the given idtype */
-        if (MUTEX_TABLE[idtype] == NULL)
+        if (impl == NULL)
         {
             break;
         }
 
         /*
-         ** initialize the pthread mutex attribute structure with default values
+         * initialize the pthread mutex attribute structure with default values
          */
         ret = pthread_mutexattr_init(&mutex_attr);
         if (ret != 0)
@@ -189,7 +235,7 @@ int32 OS_Posix_TableMutex_Init(osal_objtype_t idtype)
         }
 
         /*
-         ** Allow the mutex to use priority inheritance
+         * Allow the mutex to use priority inheritance
          */
         ret = pthread_mutexattr_setprotocol(&mutex_attr, PTHREAD_PRIO_INHERIT);
         if (ret != 0)
@@ -200,24 +246,35 @@ int32 OS_Posix_TableMutex_Init(osal_objtype_t idtype)
         }
 
         /*
-         **  Set the mutex type to RECURSIVE so a thread can do nested locks
-         **  TBD - not sure if this is really desired, but keep it for now.
+         * Use normal (faster/non-recursive) mutex implementation
+         * There should not be any instances of OSAL locking its own table more than once.
          */
-        ret = pthread_mutexattr_settype(&mutex_attr, PTHREAD_MUTEX_RECURSIVE);
+        ret = pthread_mutexattr_settype(&mutex_attr, PTHREAD_MUTEX_NORMAL);
         if (ret != 0)
         {
             OS_DEBUG("Error: pthread_mutexattr_settype failed: %s\n", strerror(ret));
             return_code = OS_ERROR;
             break;
         }
 
-        ret = pthread_mutex_init(&MUTEX_TABLE[idtype]->mutex, &mutex_attr);
+        ret = pthread_mutex_init(&impl->mutex, &mutex_attr);
         if (ret != 0)
         {
             OS_DEBUG("Error: pthread_mutex_init failed: %s\n", strerror(ret));
             return_code = OS_ERROR;
             break;
         }
+
+        /* create a condition variable with default attributes.
+         * This will be broadcast every time the object table changes */
+        ret = pthread_cond_init(&impl->cond, NULL);
+        if (ret != 0)
+        {
+            OS_DEBUG("Error: pthread_cond_init failed: %s\n", strerror(ret));
+            return_code = OS_ERROR;
+            break;
+        }
+
     } while (0);
 
     return (return_code);

src/os/rtems/src/os-impl-idmap.c

@@ -146,6 +146,33 @@ int32 OS_Unlock_Global_Impl(osal_objtype_t idtype)
     return OS_SUCCESS;
 } /* end OS_Unlock_Global_Impl */
 
+/*----------------------------------------------------------------
+ *
+ *  Function: OS_WaitForStateChange_Impl
+ *
+ *  Purpose: Implemented per internal OSAL API
+ *           See prototype for argument/return detail
+ *
+ *-----------------------------------------------------------------*/
+void OS_WaitForStateChange_Impl(osal_objtype_t idtype, uint32 attempts)
+{
+    uint32 wait_ms;
+
+    if (attempts <= 10)
+    {
+        wait_ms = attempts * attempts * 10;
+    }
+    else
+    {
+        wait_ms = 1000;
+    }
+
+    OS_Unlock_Global_Impl(idtype);
+    OS_TaskDelay(wait_ms);
+    OS_Lock_Global_Impl(idtype);
+}
+
+
 /****************************************************************************************
                                 INITIALIZATION FUNCTION
  ***************************************************************************************/

src/os/rtems/src/os-impl-tasks.c

@@ -171,6 +171,20 @@ int32 OS_TaskDelete_Impl(const OS_object_token_t *token)
     return OS_SUCCESS;
 } /* end OS_TaskDelete_Impl */
 
+/*----------------------------------------------------------------
+ *
+ * Function: OS_TaskDetach_Impl
+ *
+ *  Purpose: Implemented per internal OSAL API
+ *           See prototype for argument/return detail
+ *
+ *-----------------------------------------------------------------*/
+int32 OS_TaskDetach_Impl(const OS_object_token_t *token)
+{
+    /* No-op on RTEMS */
+    return OS_SUCCESS;
+}
+
 /*----------------------------------------------------------------
  *
  * Function: OS_TaskExit_Impl

src/os/shared/inc/os-shared-common.h

@@ -127,4 +127,20 @@ void OS_IdleLoop_Impl(void);
  ------------------------------------------------------------------*/
 void OS_ApplicationShutdown_Impl(void);
 
+/*----------------------------------------------------------------
+
+   Function: OS_WaitForStateChange_Impl
+
+   Purpose: Block the caller until some sort of change event
+   has occurred for the given object type, such as a record changing
+   state i.e. the acquisition or release of a lock/refcount from 
+   another thread.
+
+   It is not guaranteed what, if any, state change has actually 
+   occured when this function returns.  This may be implement as
+   a simple OS_TaskDelay().
+
+ ------------------------------------------------------------------*/
+void OS_WaitForStateChange_Impl(osal_objtype_t objtype, uint32 attempts);
+
 #endif /* INCLUDE_OS_SHARED_COMMON_H_ */

src/os/shared/src/osapi-idmap.c

@@ -433,9 +433,10 @@ int32 OS_ObjectIdConvertToken(OS_object_token_t *token)
             break;
         }
 
-        OS_Unlock_Global(token->obj_type);
-        OS_TaskDelay_Impl(attempts);
-        OS_Lock_Global(token->obj_type);
+        /* 
+         * Call the impl layer to wait for some sort of change to occur.
+         */
+        OS_WaitForStateChange_Impl(token->obj_type, attempts);
     }
 
     /*

src/os/vxworks/src/os-impl-tasks.c

@@ -287,6 +287,20 @@ int32 OS_TaskDelete_Impl(const OS_object_token_t *token)
 
 } /* end OS_TaskDelete_Impl */
 
+/*----------------------------------------------------------------
+ *
+ * Function: OS_TaskDetach_Impl
+ *
+ *  Purpose: Implemented per internal OSAL API
+ *           See prototype for argument/return detail
+ *
+ *-----------------------------------------------------------------*/
+int32 OS_TaskDetach_Impl(const OS_object_token_t *token)
+{
+    /* No-op on VxWorks */
+    return OS_SUCCESS;
+}
+
 /*----------------------------------------------------------------
  *
  * Function: OS_TaskExit_Impl

src/unit-test-coverage/ut-stubs/src/osapi-common-impl-stubs.c

@@ -45,3 +45,8 @@ void OS_ApplicationShutdown_Impl(void)
 {
     UT_DEFAULT_IMPL(OS_ApplicationShutdown_Impl);
 }
+
+void OS_WaitForStateChange_Impl(osal_objtype_t objtype, uint32 attempts)
+{
+    UT_DEFAULT_IMPL(OS_WaitForStateChange_Impl);
+}