Fix #642, make OS_TaskDelete synchronous

In the POSIX implementation, OS_TaskDelete was implemented in a
deferred manner - the API call was a request, and the actual
deletion occured sometime thereafter.  This is a problem if the
task is running code within a dynamically loaded module, and the
intent is to delete the task so the module can be unloaded.  In
this case the app needs to be certain that the task has actually
been deleted before unloading can be done safely.

To do this requires use of pthread_join() on POSIX which confirms
that the task has exited.  However, this is a (potentially) blocking
call, so to do this requires rework of the EXCLUSIVE lock mode
such that the OSAL lock is _not_ held during the join operation.

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

@@ -477,6 +477,16 @@ int32 OS_Posix_InternalTaskCreate_Impl(pthread_t *pthr, osal_priority_t priority
         return (OS_ERROR);
     }
 
+    /*
+    ** Set the thread to be joinable by default
+    */
+    return_code = pthread_attr_setdetachstate(&custom_attr, PTHREAD_CREATE_JOINABLE);
+    if (return_code != 0)
+    {
+        OS_DEBUG("pthread_attr_setdetachstate error in OS_TaskCreate: %s\n", strerror(return_code));
+        return (OS_ERROR);
+    }
+
     /*
     ** Test to see if the original main task scheduling priority worked.
     ** If so, then also set the attributes for this task.  Otherwise attributes
@@ -541,12 +551,6 @@ int32 OS_Posix_InternalTaskCreate_Impl(pthread_t *pthr, osal_priority_t priority
      ** Do not treat anything bad that happens after this point as fatal.
      ** The task is running, after all - better to leave well enough alone.
      */
-    return_code = pthread_detach(*pthr);
-    if (return_code != 0)
-    {
-        OS_DEBUG("pthread_detach error in OS_TaskCreate: %s\n", strerror(return_code));
-    }
-
     return_code = pthread_attr_destroy(&custom_attr);
     if (return_code != 0)
     {
@@ -583,6 +587,33 @@ int32 OS_TaskCreate_Impl(const OS_object_token_t *token, uint32 flags)
     return return_code;
 } /* end OS_TaskCreate_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)
+{
+    OS_impl_task_internal_record_t *impl;
+    int ret;
+
+    impl = OS_OBJECT_TABLE_GET(OS_impl_task_table, *token);
+
+    ret = pthread_detach(impl->id);
+
+    if (ret != 0)
+    {
+        OS_DEBUG("pthread_detach: Failed on Task ID = %lu, err = %s\n",
+                 OS_ObjectIdToInteger(OS_ObjectIdFromToken(token)), strerror(ret));
+        return OS_ERROR;
+    }
+
+    return OS_SUCCESS;
+}
+
 /*----------------------------------------------------------------
  *
  * Function: OS_TaskMatch_Impl
@@ -616,6 +647,8 @@ int32 OS_TaskMatch_Impl(const OS_object_token_t *token)
 int32 OS_TaskDelete_Impl(const OS_object_token_t *token)
 {
     OS_impl_task_internal_record_t *impl;
+    void *retval;
+    int ret;
 
     impl = OS_OBJECT_TABLE_GET(OS_impl_task_table, *token);
 
@@ -625,7 +658,35 @@ int32 OS_TaskDelete_Impl(const OS_object_token_t *token)
     ** to cancel here is that the thread ID is invalid because it already exited itself,
     ** and if that is true there is nothing wrong - everything is OK to continue normally.
     */
-    pthread_cancel(impl->id);
+    ret = pthread_cancel(impl->id);
+    if (ret != 0)
+    {
+        OS_DEBUG("pthread_cancel: Failed on Task ID = %lu, err = %s\n",
+                 OS_ObjectIdToInteger(OS_ObjectIdFromToken(token)), strerror(ret));
+
+        /* fall through (will still return OS_SUCCESS) */
+    }
+    else
+    {
+        /*
+         * Note that "pthread_cancel" is a request - and successful return above
+         * only means that the cancellation request is pending.
+         *
+         * pthread_join() will wait until the thread has actually exited.
+         *
+         * This is important for CFE, as task deletion often occurs in
+         * conjunction with an application reload - which means the next
+         * call is likely to be OS_ModuleUnload().  So is critical that all
+         * tasks potentially executing code within that module have actually
+         * been stopped - not just pending cancellation.
+         */
+        ret = pthread_join(impl->id, &retval);
+        if (ret != 0)
+        {
+            OS_DEBUG("pthread_join: Failed on Task ID = %lu, err = %s\n",
+                     OS_ObjectIdToInteger(OS_ObjectIdFromToken(token)), strerror(ret));
+        }
+    }
     return OS_SUCCESS;
 
 } /* end OS_TaskDelete_Impl */
@@ -731,6 +792,17 @@ int32 OS_TaskRegister_Impl(osal_id_t global_task_id)
 {
     int32                return_code;
     OS_U32ValueWrapper_t arg;
+    int                  old_state;
+    int                  old_type;
+
+    /*
+     * Set cancel state=ENABLED, type=DEFERRED
+     * This should be the default for new threads, but
+     * setting explicitly to be sure that a pthread_join()
+     * will work as expected in case this thread is deleted.
+     */
+    pthread_setcancelstate(PTHREAD_CANCEL_ENABLE, &old_state);
+    pthread_setcanceltype(PTHREAD_CANCEL_DEFERRED, &old_type);
 
     arg.opaque_arg = 0;
     arg.id         = global_task_id;

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

@@ -386,7 +386,7 @@ int32 OS_TimeBaseCreate_Impl(const OS_object_token_t *token)
          */
         for (idx = 0; idx < OS_MAX_TIMEBASES; ++idx)
         {
-            if (OS_ObjectIdDefined(OS_global_timebase_table[idx].active_id) &&
+            if (OS_ObjectIdIsValid(OS_global_timebase_table[idx].active_id) &&
                 OS_impl_timebase_table[idx].assigned_signal != 0)
             {
                 sigaddset(&local->sigset, OS_impl_timebase_table[idx].assigned_signal);

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-idmap.h

@@ -30,8 +30,6 @@
 
 #include <os-shared-globaldefs.h>
 
-#define OS_OBJECT_EXCL_REQ_FLAG 0x0001
-
 #define OS_OBJECT_ID_RESERVED ((osal_id_t) {0xFFFFFFFF})
 
 /*
@@ -43,7 +41,6 @@ struct OS_common_record
     osal_id_t   active_id;
     osal_id_t   creator;
     uint16      refcount;
-    uint16      flags;
 };
 
 /*
@@ -214,6 +211,26 @@ static inline void OS_ObjectIdCompose_Impl(osal_objtype_t idtype, uint32 idseria
     *result = OS_ObjectIdFromInteger((idtype << OS_OBJECT_TYPE_SHIFT) | idserial);
 }
 
+/*-------------------------------------------------------------------------------------*/
+/**
+ * @brief Check if an object ID represents a valid/active value.
+ *
+ * This tests that the ID value is within the range specifically used by
+ * valid OSAL IDs. This is smaller than the set of defined IDs.
+ *
+ * For example, the value of OS_OBJECT_ID_RESERVED is defined but not valid.
+ * So while OS_ObjectIdDefined() will match entries being actively created or
+ * deleted, OS_ObjectIdIsValid() will not.
+ *
+ * @param[in]   object_id The object ID
+ * @returns     true if table entry is valid
+ */
+static inline bool OS_ObjectIdIsValid(osal_id_t object_id)
+{
+    osal_objtype_t objtype = OS_ObjectIdToType_Impl(object_id);
+    return (objtype > OS_OBJECT_TYPE_UNDEFINED && objtype < OS_OBJECT_TYPE_USER);
+}
+
 /*----------------------------------------------------------------
    Function: OS_GetMaxForObjectType
 

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

@@ -94,6 +94,16 @@ int32 OS_TaskMatch_Impl(const OS_object_token_t *token);
  ------------------------------------------------------------------*/
 int32 OS_TaskCreate_Impl(const OS_object_token_t *token, uint32 flags);
 
+/*----------------------------------------------------------------
+   Function: OS_TaskDetach_Impl
+
+    Purpose: Sets the thread so that the OS resources associated with the task
+             will be released when the thread exits itself
+
+    Returns: OS_SUCCESS on success, or relevant error code
+ ------------------------------------------------------------------*/
+int32 OS_TaskDetach_Impl(const OS_object_token_t *token);
+
 /*----------------------------------------------------------------
    Function: OS_TaskDelete_Impl