Merge pull request #1050 from nasa/integration-candidate

osal Integration candidate: 2021-05-25

README.md

@@ -11,6 +11,13 @@ The autogenerated OSAL user's guide can be viewed at <https://github.com/nasa/cF
 
 ## Version History
 
+### Development Build: v5.1.0-rc1+dev458
+
+- Adds Counting Semaphore Test with timeouts
+- Removes chance of deadlock by ensuring OS_TaskGetId returns the task ID it knows about, regardless of whether the task is pending cancellation or not. Defers cancellation of the task while the BSP is locked, ensure it reaches the unlock, then restores the previous cancel state.
+- Fixes a bogus debug message about unlocking from the wrong task if the task is pending delete.
+See <https://github.com/nasa/osal/pull/1050> and <https://github.com/nasa/cFS/pull/260>
+
 ### Development Build: v5.1.0-rc1+dev452
 
 - Makes filenames better match terms used in implementation.

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);
+ }
 }
 
 /* ---------------------------------------------------------

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;
 
 /*

src/os/inc/osapi-version.h

@@ -34,7 +34,7 @@
 /*
  * Development Build Macro Definitions
  */
-#define OS_BUILD_NUMBER 452
+#define OS_BUILD_NUMBER 458
 #define OS_BUILD_BASELINE "v5.1.0-rc1"
 
 /*

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 */
 

src/tests/count-sem-timeout-test/count-sem-timeout-test.c

@@ -0,0 +1,285 @@
+/*
+ * NASA Docket No. GSC-18,370-1, and identified as "Operating System Abstraction Layer"
+ *
+ * Copyright (c) 2019 United States Government as represented by
+ * the Administrator of the National Aeronautics and Space Administration.
+ * All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http:https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/*
+** Counting Semaphore Test with timeouts
+*/
+#include <stdio.h>
+#include "common_types.h"
+#include "osapi.h"
+#include "utassert.h"
+#include "uttest.h"
+#include "utbsp.h"
+
+/* Define setup and check functions for UT assert */
+void CountSemTimeoutSetup(void);
+void CountSemTimeoutCheck(void);
+
+#define TASK_STACK_SIZE 4096
+#define TASK_1_PRIORITY 100
+#define TASK_2_PRIORITY 110
+#define TASK_3_PRIORITY 120
+
+uint32 task_1_stack[TASK_STACK_SIZE];
+osal_id_t task_1_id;
+uint32 task_1_failures;
+uint32 task_1_work;
+
+uint32 task_2_stack[TASK_STACK_SIZE];
+osal_id_t task_2_id;
+uint32 task_2_failures;
+uint32 task_2_work;
+uint32 task_2_timeouts;
+
+uint32 task_3_stack[TASK_STACK_SIZE];
+osal_id_t task_3_id;
+uint32 task_3_failures;
+uint32 task_3_work;
+uint32 task_3_timeouts;
+
+osal_id_t count_sem_id;
+
+void task_1(void)
+{
+ uint32 status;
+
+ OS_printf("Starting task 1\n");
+
+ while (1)
+ {
+ OS_TaskDelay(2000);
+
+ OS_printf("TASK 1: Giving the counting semaphore 1\n");
+ status = OS_CountSemGive(count_sem_id);
+ if (status != OS_SUCCESS)
+ {
+ ++task_1_failures;
+ OS_printf("TASK 1: Error calling OS_CountSemGive 1: %d\n", (int)status);
+ }
+ else
+ {
+ ++task_1_work;
+ OS_printf("TASK 1: Counting Sem Give 1 complete\n");
+ }
+
+ OS_TaskDelay(100);
+
+ OS_printf("TASK 1: Giving the counting semaphore 2\n");
+ status = OS_CountSemGive(count_sem_id);
+ if (status != OS_SUCCESS)
+ {
+ ++task_1_failures;
+ OS_printf("TASK 1: Error calling OS_CountSemGive 2: %d\n", (int)status);
+ }
+ else
+ {
+ ++task_1_work;
+ OS_printf("TASK 1: Counting Sem Give 2 complete\n");
+ }
+
+ OS_TaskDelay(100);
+
+ OS_printf("TASK 1: Giving the counting semaphore 3\n");
+ status = OS_CountSemGive(count_sem_id);
+ if (status != OS_SUCCESS)
+ {
+ ++task_1_failures;
+ OS_printf("TASK 1: Error calling OS_CountSemGive 3: %d\n", (int)status);
+ }
+ else
+ {
+ ++task_1_work;
+ OS_printf("TASK 1: Counting Sem Give 3 complete\n");
+ }
+ }
+}
+
+void task_2(void)
+{
+ uint32 status;
+
+ OS_printf("Starting task 2\n");
+
+ while (1)
+ {
+ OS_TaskDelay(2000);
+ OS_printf("TASK 2: Waiting on the semaphore\n");
+ status = OS_CountSemTimedWait(count_sem_id, 5000);
+ if (status == OS_SUCCESS)
+ {
+ ++task_2_work;
+ OS_printf("TASK 2: grabbed Counting Sem\n");
+ }
+ else if (status == OS_SEM_TIMEOUT)
+ {
+ OS_printf("TASK 2: Timeout on OS_CountSemTimedWait\n");
+ ++task_2_timeouts;
+ }
+ else
+ {
+ ++task_2_failures;
+ OS_printf("TASK 2: Error calling OS_CountSemTake: %d\n", (int)status);
+ }
+ }
+}
+
+void task_3(void)
+{
+ uint32 status;
+
+ OS_printf("Starting task 3\n");
+
+ while (1)
+ {
+ OS_TaskDelay(250);
+ OS_printf("TASK 3: Waiting on the semaphore\n");
+ status = OS_CountSemTimedWait(count_sem_id, 500);
+ if (status == OS_SUCCESS)
+ {
+ ++task_3_work;
+ OS_printf("TASK 3: grabbed Counting Sem\n");
+ }
+ else if (status == OS_SEM_TIMEOUT)
+ {
+ OS_printf("TASK 3: Timeout on OS_CountSemTimedWait\n");
+ ++task_3_timeouts;
+ }
+ else
+ {
+ ++task_3_failures;
+ OS_printf("TASK 3: Error calling OS_CountSemTake: %d\n", (int)status);
+ }
+ }
+}
+
+void UtTest_Setup(void)
+{
+ if (OS_API_Init() != OS_SUCCESS)
+ {
+ UtAssert_Abort("OS_API_Init() failed");
+ }
+
+ /* the test should call OS_API_Teardown() before exiting */
+ UtTest_AddTeardown(OS_API_Teardown, "Cleanup");
+
+ /*
+ * Register the test setup and check routines in UT assert
+ */
+ UtTest_Add(CountSemTimeoutCheck, CountSemTimeoutSetup, NULL, "CountSemTest");
+}
+
+void CountSemTimeoutSetup(void)
+{
+ uint32 status;
+ int i;
+
+ task_1_failures = 0;
+ task_2_failures = 0;
+ task_3_failures = 0;
+ task_1_work = 0;
+ task_2_work = 0;
+ task_3_work = 0;
+ task_2_timeouts = 0;
+ task_3_timeouts = 0;
+
+ /*
+ ** Create the Counting semaphore
+ */
+ status = OS_CountSemCreate(&count_sem_id, "CountSem1", 2, 0);
+ UtAssert_True(status == OS_SUCCESS, "CountSem1 create Id=%lx Rc=%d", OS_ObjectIdToInteger(count_sem_id),
+ (int)status);
+
+ /*
+ ** Take the semaphore so the value is 0 and the next SemTake call should block
+ */
+ for (i = 0; i < 2; i++)
+ {
+ status = OS_CountSemTake(count_sem_id);
+ UtAssert_True(status == OS_SUCCESS, "CountSem1 take Rc=%d", (int)status);
+ }
+
+ /*
+ ** Create the tasks
+ */
+ status = OS_TaskCreate(&task_1_id, "Task 1", task_1, OSAL_STACKPTR_C(task_1_stack), sizeof(task_1_stack),
+ OSAL_PRIORITY_C(TASK_1_PRIORITY), 0);
+ UtAssert_True(status == OS_SUCCESS, "Task 1 create Id=%lx Rc=%d", OS_ObjectIdToInteger(task_1_id), (int)status);
+
+ status = OS_TaskCreate(&task_2_id, "Task 2", task_2, OSAL_STACKPTR_C(task_2_stack), sizeof(task_2_stack),
+ OSAL_PRIORITY_C(TASK_2_PRIORITY), 0);
+ UtAssert_True(status == OS_SUCCESS, "Task 2 create Id=%lx Rc=%d", OS_ObjectIdToInteger(task_2_id), (int)status);
+
+ status = OS_TaskCreate(&task_3_id, "Task 3", task_3, OSAL_STACKPTR_C(task_3_stack), sizeof(task_3_stack),
+ OSAL_PRIORITY_C(TASK_3_PRIORITY), 0);
+ UtAssert_True(status == OS_SUCCESS, "Task 3 create Id=%lx Rc=%d", OS_ObjectIdToInteger(task_3_id), (int)status);
+
+ /*
+ * Time-limited execution
+ */
+ while (task_1_work < 10)
+ {
+ OS_TaskDelay(100);
+ }
+
+ /*
+ * Delete the tasks
+ */
+ status = OS_TaskDelete(task_1_id);
+ UtAssert_True(status == OS_SUCCESS, "Task 1 delete Rc=%d", (int)status);
+ status = OS_TaskDelete(task_2_id);
+ UtAssert_True(status == OS_SUCCESS, "Task 2 delete Rc=%d", (int)status);
+ status = OS_TaskDelete(task_3_id);
+ UtAssert_True(status == OS_SUCCESS, "Task 3 delete Rc=%d", (int)status);
+}
+
+void CountSemTimeoutCheck(void)
+{
+ uint32 task_2_timeouts_expected = 0;
+ uint32 task_3_timeouts_expected = 8;
+
+ /* Task 2 and 3 should have both executed */
+ UtAssert_True(task_2_work != 0, "Task 2 work counter = %u", (unsigned int)task_2_work);
+ UtAssert_True(task_3_work != 0, "Task 3 work counter = %u", (unsigned int)task_3_work);
+
+ /*
+ * The sum of task 2 and task 3 work (consumer) cannot be greater than task 1 (the producer)
+ * Add a fudge factor of +/- 2 to help avoid false failures due to scheduling
+ */
+ UtAssert_True((task_2_work + task_3_work) <= (task_1_work + 2), "Task 2+3 work < %u",
+ (unsigned int)(task_1_work + 2));
+ UtAssert_True((task_2_work + task_3_work) >= (task_1_work - 2), "Task 2+3 work > %u",
+ (unsigned int)(task_1_work - 2));
+
+ /*
+ * Task 2 should never timeout
+ * Task 3 should timeout 8 times, include a fudge factor of 1 to account for potential scheduling
+ */
+ UtAssert_True(task_2_timeouts == task_2_timeouts_expected, "Task 2 timeout counter = %u",
+ (unsigned int)task_2_timeouts);
+ UtAssert_True(task_3_timeouts >= (task_3_timeouts_expected - 1), "Task 3 timeout counter %u >= %u",
+ (unsigned int)task_3_timeouts, (unsigned int)(task_3_timeouts_expected - 1));
+ UtAssert_True(task_3_timeouts <= (task_3_timeouts_expected + 1), "Task 3 timeout counter %u <= %u",
+ (unsigned int)task_3_timeouts, (unsigned int)(task_3_timeouts_expected + 1));
+
+ /* None of the tasks should have any failures in their own counters */
+ UtAssert_True(task_1_failures == 0, "Task 1 failures = %u", (unsigned int)task_1_failures);
+ UtAssert_True(task_2_failures == 0, "Task 2 failures = %u", (unsigned int)task_2_failures);
+ UtAssert_True(task_3_failures == 0, "Task 3 failures = %u", (unsigned int)task_3_failures);
+}

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)