From eadc3e65027de18ee84e37f51be3b0fa9e149dbd Mon Sep 17 00:00:00 2001
From: Joseph Hickey <joseph.p.hickey@nasa.gov>
Date: Wed, 19 May 2021 15:32:41 -0400
Subject: [PATCH] Fix #1027, defer cancellation when BSP locked

Resolves two related issues:
- OS_TaskGetId does not return a valid value for tasks where cancellation
  is pending, but they are still running.  This in turn is likely to trigger
  other (bogus) debug checks which invoke OS_DEBUG and in turn do console writes.
- The console write itself is a cancellation point, which is now done while
  holding a BSP mutex.  If canceled here, then the mutex is not released.

Solution is in two parts:
- OS_TaskGetId should return the task ID it knows about, regardless of whether
  the task is pending cancellation or not.
- Defer cancellation of the task while the BSP is locked, ensure it reaches the
  unlock, then restore the previous cancel state.
---
 src/bsp/generic-linux/src/bsp_start.c              | 14 ++++++++++++++
 .../generic-linux/src/generic_linux_bsp_internal.h |  1 +
 src/os/shared/src/osapi-task.c                     | 10 +---------
 .../shared/src/coveragetest-task.c                 |  4 ----
 4 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/src/bsp/generic-linux/src/bsp_start.c b/src/bsp/generic-linux/src/bsp_start.c
index 4900f2259..1a703da4c 100644
--- a/src/bsp/generic-linux/src/bsp_start.c
+++ b/src/bsp/generic-linux/src/bsp_start.c
@@ -110,6 +110,15 @@ void OS_BSP_Lock_Impl(void)
     {
         BSP_DEBUG("pthread_mutex_lock: %s\n", strerror(status));
     }
+    else
+    {
+        /* 
+         * Temporarily Disable/Defer thread cancellation.
+         * Note that OS_BSP_ConsoleOutput_Impl() calls write() which is a cancellation point.
+         * So if this calling task is canceled, it risks leaving the BSP locked.
+         */
+        pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &OS_BSP_GenericLinuxGlobal.AccessCancelState);
+    }
 }
 
 /*----------------------------------------------------------------
@@ -125,6 +134,11 @@ void OS_BSP_Unlock_Impl(void)
     {
         BSP_DEBUG("pthread_mutex_unlock: %s\n", strerror(status));
     }
+    else
+    {
+        /* Restore previous cancelability state */
+        pthread_setcancelstate(OS_BSP_GenericLinuxGlobal.AccessCancelState, NULL);
+    }
 }
 
 /* ---------------------------------------------------------
diff --git a/src/bsp/generic-linux/src/generic_linux_bsp_internal.h b/src/bsp/generic-linux/src/generic_linux_bsp_internal.h
index d61a1e478..4c51c5122 100644
--- a/src/bsp/generic-linux/src/generic_linux_bsp_internal.h
+++ b/src/bsp/generic-linux/src/generic_linux_bsp_internal.h
@@ -42,6 +42,7 @@ typedef struct
 {
     bool            EnableTermControl; /**< Will be set "true" when invoked from a TTY device, false otherwise */
     pthread_mutex_t AccessMutex;
+    int             AccessCancelState;
 } OS_BSP_GenericLinuxGlobalData_t;
 
 /*
diff --git a/src/os/shared/src/osapi-task.c b/src/os/shared/src/osapi-task.c
index bbf817aae..9124b1349 100644
--- a/src/os/shared/src/osapi-task.c
+++ b/src/os/shared/src/osapi-task.c
@@ -339,18 +339,10 @@ int32 OS_TaskSetPriority(osal_id_t task_id, osal_priority_t new_priority)
  *-----------------------------------------------------------------*/
 osal_id_t OS_TaskGetId(void)
 {
-    OS_object_token_t token;
-    osal_id_t         task_id;
+    osal_id_t task_id;
 
     task_id = OS_TaskGetId_Impl();
 
-    /* Confirm the task master table entry matches the expected.
-     * If not it means we have some stale/leftover value */
-    if (OS_ObjectIdGetById(OS_LOCK_MODE_NONE, LOCAL_OBJID_TYPE, task_id, &token) != OS_SUCCESS)
-    {
-        task_id = OS_OBJECT_ID_UNDEFINED;
-    }
-
     return (task_id);
 } /* end OS_TaskGetId */
 
diff --git a/src/unit-test-coverage/shared/src/coveragetest-task.c b/src/unit-test-coverage/shared/src/coveragetest-task.c
index 1da6564aa..58f51904f 100644
--- a/src/unit-test-coverage/shared/src/coveragetest-task.c
+++ b/src/unit-test-coverage/shared/src/coveragetest-task.c
@@ -197,10 +197,6 @@ void Test_OS_TaskGetId(void)
     UT_SetDefaultReturnValue(UT_KEY(OS_TaskGetId_Impl), idbuf.val);
     objid = OS_TaskGetId();
     OSAPI_TEST_OBJID(objid, ==, idbuf.id);
-
-    UT_SetDefaultReturnValue(UT_KEY(OS_ObjectIdGetById), OS_ERROR);
-    objid = OS_TaskGetId();
-    OSAPI_TEST_OBJID(objid, ==, OS_OBJECT_ID_UNDEFINED);
 }
 
 void Test_OS_TaskGetIdByName(void)