Fix nasa#871, allow OSAL re-initialization

Replaces the separate "Initialized" and "Shutdown" flags with a single
state flag.  This simplifies things and makes for a single source
of truth for the state of OSAL globally.

In particular this allows for:
- Multiple invocations of OS_API_Init() - subsequent calls can be ignored
- Deleting of any internal objects that did get created if OS_API_Init() fails
  (leaves system in same state as when it started)
- Allows Re-initialization of OSAL after OS_ApplicationShutdown() - may be
  relevant when running unit tests several times without rebooting.

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

@@ -100,7 +100,7 @@ static void *OS_ConsoleTask_Entry(void *arg)
         local = OS_OBJECT_TABLE_GET(OS_impl_console_table, token);
 
         /* Loop forever (unless shutdown is set) */
-        while (OS_SharedGlobalVars.ShutdownFlag != OS_SHUTDOWN_MAGIC_NUMBER)
+        while (OS_SharedGlobalVars.GlobalState != OS_SHUTDOWN_MAGIC_NUMBER)
         {
             OS_ConsoleOutput_Impl(&token);
             sem_wait(&local->data_sem);

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

@@ -123,7 +123,7 @@ static void OS_ConsoleTask_Entry(rtems_task_argument arg)
         local = OS_OBJECT_TABLE_GET(OS_impl_console_table, token);
 
         /* Loop forever (unless shutdown is set) */
-        while (OS_SharedGlobalVars.ShutdownFlag != OS_SHUTDOWN_MAGIC_NUMBER)
+        while (OS_SharedGlobalVars.GlobalState != OS_SHUTDOWN_MAGIC_NUMBER)
         {
             OS_ConsoleOutput_Impl(&token);
             rtems_semaphore_obtain(local->data_sem, RTEMS_WAIT, RTEMS_NO_TIMEOUT);

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

@@ -32,29 +32,36 @@
 #include "os-shared-globaldefs.h"
 
 /*
- * A "magic number" that when written to the "ShutdownFlag" member
- * of the global state structure indicates an active shutdown request.
+ * Flag values for the "GlobalState" member the global state structure
  */
-#define OS_SHUTDOWN_MAGIC_NUMBER 0xABADC0DE
+#define OS_INIT_MAGIC_NUMBER     0xBE57C0DE /**< Indicates that OS_API_Init() has been successfully run */
+#define OS_SHUTDOWN_MAGIC_NUMBER 0xABADC0DE /**< Indicates that a system shutdown request is pending */
 
 /* Global variables that are common between implementations */
 struct OS_shared_global_vars
 {
-    bool Initialized;
+    /*
+     * Tracks whether OS_API_Init() has been called or if
+     * there is a shutdown request pending.
+     *
+     * After boot/first startup this should have 0 (from BSS clearing)
+     * After OS_API_Init() is called this has OS_INIT_MAGIC_NUMBER
+     * After OS_ApplicationShutdown() this has OS_SHUTDOWN_MAGIC_NUMBER
+     */
+    volatile uint32 GlobalState;
 
     /*
      * The console device ID used for OS_printf() calls
      */
     osal_id_t PrintfConsoleId;
 
     /*
-     * PrintfEnabled and ShutdownFlag are marked "volatile"
+     * PrintfEnabled and GlobalState are marked "volatile"
      * because they are updated and read by different threads
      */
-    volatile bool   PrintfEnabled;
-    volatile uint32 ShutdownFlag;
-    uint32          MicroSecPerTick;
-    uint32          TicksPerSecond;
+    volatile bool PrintfEnabled;
+    uint32        MicroSecPerTick;
+    uint32        TicksPerSecond;
 
     /*
      * The event handler is an application-defined callback

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

@@ -58,9 +58,8 @@
 #include "os-shared-time.h"
 
 OS_SharedGlobalVars_t OS_SharedGlobalVars = {
-    .Initialized     = false,
+    .GlobalState     = 0,
     .PrintfEnabled   = false,
-    .ShutdownFlag    = 0,
     .MicroSecPerTick = 0, /* invalid, _must_ be set by implementation init */
     .TicksPerSecond  = 0, /* invalid, _must_ be set by implementation init */
     .EventHandler    = NULL,
@@ -112,13 +111,29 @@ int32 OS_API_Init(void)
     osal_objtype_t idtype;
     uint32         microSecPerSec;
 
-    if (OS_SharedGlobalVars.Initialized != false)
+    /*
+     * If OSAL is already initialized, not really a big issue, just return.
+     * This is not typically expected though, so its worth a debug statement.
+     *
+     * However this can validly occur when running tests on some platforms
+     * without a reset/reload between invocations.
+     */
+    if (OS_SharedGlobalVars.GlobalState == OS_INIT_MAGIC_NUMBER)
     {
-        OS_DEBUG("WARNING: BUG - initialization function called multiple times\n");
-        return OS_ERROR;
+        OS_DEBUG("NOTE: ignored redundant OS_API_Init() call\n");
+        return OS_SUCCESS;
     }
 
-    OS_SharedGlobalVars.Initialized = true;
+    /* Wipe global state structure to be sure everything is clean */
+    memset(&OS_SharedGlobalVars, 0, sizeof(OS_SharedGlobalVars));
+
+    /* Reset debug to default level if enabled */
+#if defined(OSAL_CONFIG_DEBUG_PRINTF)
+    OS_SharedGlobalVars.DebugLevel = 1;
+#endif
+
+    /* Set flag that says OSAL has been initialized */
+    OS_SharedGlobalVars.GlobalState = OS_INIT_MAGIC_NUMBER;
 
     /* Initialize the common table that everything shares */
     return_code = OS_ObjectIdInit();
@@ -216,6 +231,18 @@ int32 OS_API_Init(void)
                  (long)OS_SharedGlobalVars.TicksPerSecond);
     }
 
+    if (return_code != OS_SUCCESS)
+    {
+        /*
+         * Some part of init failed, so set global flag that says OSAL is in shutdown state.
+         *
+         * In particular if certain internal resources (such as the console utility task)
+         * were created, this should cause those tasks to self-exit such that the system
+         * is ultimately returned to the same state it started in.
+         */
+        OS_SharedGlobalVars.GlobalState = OS_SHUTDOWN_MAGIC_NUMBER;
+    }
+
     return (return_code);
 } /* end OS_API_Init */
 
@@ -359,7 +386,7 @@ void OS_IdleLoop()
      * In most "real" embedded systems, this will never happen.
      * However it will happen in debugging situations (CTRL+C, etc).
      */
-    while (OS_SharedGlobalVars.ShutdownFlag != OS_SHUTDOWN_MAGIC_NUMBER)
+    while (OS_SharedGlobalVars.GlobalState != OS_SHUTDOWN_MAGIC_NUMBER)
     {
         OS_IdleLoop_Impl();
     }
@@ -377,7 +404,7 @@ void OS_ApplicationShutdown(uint8 flag)
 {
     if (flag == true)
     {
-        OS_SharedGlobalVars.ShutdownFlag = OS_SHUTDOWN_MAGIC_NUMBER;
+        OS_SharedGlobalVars.GlobalState = OS_SHUTDOWN_MAGIC_NUMBER;
     }
 
     /*

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

@@ -324,7 +324,11 @@ int32 OS_ObjectIdTransactionInit(OS_lock_mode_t lock_mode, osal_objtype_t idtype
 {
     memset(token, 0, sizeof(*token));
 
-    if (OS_SharedGlobalVars.Initialized == false)
+    /*
+     * Confirm that OSAL has been fully initialized before allowing any transactions
+     */
+    if (OS_SharedGlobalVars.GlobalState != OS_INIT_MAGIC_NUMBER &&
+        OS_SharedGlobalVars.GlobalState != OS_SHUTDOWN_MAGIC_NUMBER)
     {
         return OS_ERROR;
     }
@@ -333,7 +337,7 @@ int32 OS_ObjectIdTransactionInit(OS_lock_mode_t lock_mode, osal_objtype_t idtype
      * only "exclusive" locks allowed after shutdown request (this is mode used for delete).
      * All regular ops will be blocked.
      */
-    if (OS_SharedGlobalVars.ShutdownFlag == OS_SHUTDOWN_MAGIC_NUMBER && lock_mode != OS_LOCK_MODE_EXCLUSIVE)
+    if (OS_SharedGlobalVars.GlobalState == OS_SHUTDOWN_MAGIC_NUMBER && lock_mode != OS_LOCK_MODE_EXCLUSIVE)
     {
         return OS_ERR_INCORRECT_OBJ_STATE;
     }
@@ -1211,7 +1215,10 @@ int32 OS_ObjectIdAllocateNew(osal_objtype_t idtype, const char *name, OS_object_
 {
     int32 return_code;
 
-    if (OS_SharedGlobalVars.ShutdownFlag == OS_SHUTDOWN_MAGIC_NUMBER)
+    /*
+     * No new objects can be created after Shutdown request
+     */
+    if (OS_SharedGlobalVars.GlobalState == OS_SHUTDOWN_MAGIC_NUMBER)
     {
         return OS_ERR_INCORRECT_OBJ_STATE;
     }

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

@@ -258,7 +258,7 @@ void OS_printf(const char *String, ...)
 
     BUGCHECK((String) != NULL, )
 
-    if (!OS_SharedGlobalVars.Initialized)
+    if (OS_SharedGlobalVars.GlobalState != OS_INIT_MAGIC_NUMBER)
     {
         /*
          * Catch some historical mis-use of the OS_printf() call.
@@ -277,7 +277,7 @@ void OS_printf(const char *String, ...)
          * If debugging is not enabled, then this message will be silently
          * discarded.
          */
-        OS_DEBUG("BUG: OS_printf() called before init: %s", String);
+        OS_DEBUG("BUG: OS_printf() called when OSAL not initialized: %s", String);
     }
     else if (OS_SharedGlobalVars.PrintfEnabled)
     {

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

@@ -107,7 +107,7 @@ int OS_VxWorks_ConsoleTask_Entry(int arg)
         local = OS_OBJECT_TABLE_GET(OS_impl_console_table, token);
 
         /* Loop forever (unless shutdown is set) */
-        while (OS_SharedGlobalVars.ShutdownFlag != OS_SHUTDOWN_MAGIC_NUMBER)
+        while (OS_SharedGlobalVars.GlobalState != OS_SHUTDOWN_MAGIC_NUMBER)
         {
             OS_ConsoleOutput_Impl(&token);
             if (semTake(local->datasem, WAIT_FOREVER) == ERROR)

src/unit-test-coverage/shared/src/coveragetest-common.c

@@ -95,34 +95,37 @@ void Test_OS_API_Init(void)
     /* Execute Test */
     Test_MicroSecPerTick            = 0;
     Test_TicksPerSecond             = 0;
-    OS_SharedGlobalVars.Initialized = false;
+    OS_SharedGlobalVars.GlobalState = 0;
     OSAPI_TEST_FUNCTION_RC(OS_API_Init(), OS_ERROR);
+    UtAssert_UINT32_EQ(OS_SharedGlobalVars.GlobalState, OS_SHUTDOWN_MAGIC_NUMBER);
 
     Test_MicroSecPerTick            = 1000;
     Test_TicksPerSecond             = 1000;
-    OS_SharedGlobalVars.Initialized = false;
+    OS_SharedGlobalVars.GlobalState = 0;
     OSAPI_TEST_FUNCTION_RC(OS_API_Init(), OS_SUCCESS);
 
     Test_MicroSecPerTick            = 1000;
     Test_TicksPerSecond             = 1001;
-    OS_SharedGlobalVars.Initialized = false;
+    OS_SharedGlobalVars.GlobalState = 0;
     OSAPI_TEST_FUNCTION_RC(OS_API_Init(), OS_SUCCESS);
+    UtAssert_UINT32_EQ(OS_SharedGlobalVars.GlobalState, OS_INIT_MAGIC_NUMBER);
 
-    /* Second call should return ERROR */
-    OSAPI_TEST_FUNCTION_RC(OS_API_Init(), OS_ERROR);
+    /* Second call should return SUCCESS (but is a no-op) */
+    OSAPI_TEST_FUNCTION_RC(OS_API_Init(), OS_SUCCESS);
+    UtAssert_UINT32_EQ(OS_SharedGlobalVars.GlobalState, OS_INIT_MAGIC_NUMBER);
 
     /* other error paths */
-    OS_SharedGlobalVars.Initialized = false;
+    OS_SharedGlobalVars.GlobalState = 0;
     UT_SetDefaultReturnValue(UT_KEY(OS_ObjectIdInit), -222);
     OSAPI_TEST_FUNCTION_RC(OS_API_Init(), -222);
     UT_ResetState(UT_KEY(OS_ObjectIdInit));
 
-    OS_SharedGlobalVars.Initialized = false;
+    OS_SharedGlobalVars.GlobalState = 0;
     UT_SetDefaultReturnValue(UT_KEY(OS_API_Impl_Init), -333);
     OSAPI_TEST_FUNCTION_RC(OS_API_Init(), -333);
     UT_ResetState(UT_KEY(OS_API_Impl_Init));
 
-    OS_SharedGlobalVars.Initialized = false;
+    OS_SharedGlobalVars.GlobalState = 0;
     UT_SetDefaultReturnValue(UT_KEY(OS_TaskAPI_Init), -444);
     OSAPI_TEST_FUNCTION_RC(OS_API_Init(), -444);
     UT_ResetState(UT_KEY(OS_TaskAPI_Init));
@@ -255,6 +258,8 @@ void Test_OS_IdleLoopAndShutdown(void)
      */
     uint32 CallCount = 0;
 
+    OS_SharedGlobalVars.GlobalState = OS_INIT_MAGIC_NUMBER;
+
     UT_SetHookFunction(UT_KEY(OS_IdleLoop_Impl), SetShutdownFlagHook, NULL);
     OS_IdleLoop();
 

src/unit-test-coverage/shared/src/coveragetest-idmap.c

@@ -482,13 +482,13 @@ void Test_OS_ObjectIdGetById(void)
     OS_object_token_t   token2;
 
     /* verify that the call returns ERROR when not initialized */
-    OS_SharedGlobalVars.Initialized = false;
+    OS_SharedGlobalVars.GlobalState = 0;
     actual                          = OS_ObjectIdGetById(OS_LOCK_MODE_NONE, 0, OS_OBJECT_ID_UNDEFINED, &token1);
     expected                        = OS_ERROR;
     UtAssert_True(actual == expected, "OS_ObjectIdGetById(uninitialized) (%ld) == OS_ERROR", (long)actual);
 
     /* set "true" for the remainder of tests */
-    OS_SharedGlobalVars.Initialized = true;
+    OS_SharedGlobalVars.GlobalState = OS_INIT_MAGIC_NUMBER;
 
     OS_ObjectIdCompose_Impl(OS_OBJECT_TYPE_OS_TASK, 1000, &refobjid);
     OS_ObjectIdToArrayIndex(OS_OBJECT_TYPE_OS_TASK, refobjid, &local_idx);
@@ -516,17 +516,16 @@ void Test_OS_ObjectIdGetById(void)
     UtAssert_True(rptr->refcount == 0, "refcount (%u) == 0", (unsigned int)rptr->refcount);
 
     /* attempt to get non-exclusive lock during shutdown should fail */
-    OS_SharedGlobalVars.ShutdownFlag = OS_SHUTDOWN_MAGIC_NUMBER;
-    expected                         = OS_ERR_INCORRECT_OBJ_STATE;
-    actual                           = OS_ObjectIdGetById(OS_LOCK_MODE_NONE, OS_OBJECT_TYPE_OS_TASK, refobjid, &token1);
+    OS_SharedGlobalVars.GlobalState = OS_SHUTDOWN_MAGIC_NUMBER;
+    expected                        = OS_ERR_INCORRECT_OBJ_STATE;
+    actual                          = OS_ObjectIdGetById(OS_LOCK_MODE_NONE, OS_OBJECT_TYPE_OS_TASK, refobjid, &token1);
     UtAssert_True(actual == expected, "OS_ObjectIdGetById() (%ld) == OS_ERR_INCORRECT_OBJ_STATE", (long)actual);
-    OS_SharedGlobalVars.ShutdownFlag = 0;
+    OS_SharedGlobalVars.GlobalState = OS_INIT_MAGIC_NUMBER;
 
     /* attempt to get lock for invalid type object should fail */
     expected = OS_ERR_INCORRECT_OBJ_TYPE;
     actual   = OS_ObjectIdGetById(OS_LOCK_MODE_NONE, 0xFFFF, refobjid, &token1);
     UtAssert_True(actual == expected, "OS_ObjectIdGetById() (%ld) == OS_ERR_INCORRECT_OBJ_TYPE", (long)actual);
-    OS_SharedGlobalVars.ShutdownFlag = 0;
 
     /* clear out state entry */
     memset(&OS_global_task_table[local_idx], 0, sizeof(OS_global_task_table[local_idx]));
@@ -702,10 +701,10 @@ void Test_OS_ObjectIdAllocateNew(void)
     actual   = OS_ObjectIdGetByName(OS_LOCK_MODE_NONE, OS_OBJECT_TYPE_OS_TASK, "UT_alloc", &token);
     UtAssert_True(actual == expected, "OS_ObjectIdGetByName() (%ld) == OS_ERR_INCORRECT_OBJ_STATE", (long)actual);
 
-    OS_SharedGlobalVars.ShutdownFlag = OS_SHUTDOWN_MAGIC_NUMBER;
-    expected                         = OS_ERR_INCORRECT_OBJ_STATE;
-    actual                           = OS_ObjectIdAllocateNew(OS_OBJECT_TYPE_OS_TASK, "UT_alloc", &token);
-    OS_SharedGlobalVars.ShutdownFlag = 0;
+    OS_SharedGlobalVars.GlobalState = OS_SHUTDOWN_MAGIC_NUMBER;
+    expected                        = OS_ERR_INCORRECT_OBJ_STATE;
+    actual                          = OS_ObjectIdAllocateNew(OS_OBJECT_TYPE_OS_TASK, "UT_alloc", &token);
+    OS_SharedGlobalVars.GlobalState = OS_INIT_MAGIC_NUMBER;
     UtAssert_True(actual == expected, "OS_ObjectIdAllocate() (%ld) == OS_ERR_INCORRECT_OBJ_STATE", (long)actual);
 
     expected = OS_ERR_INCORRECT_OBJ_TYPE;
@@ -773,8 +772,7 @@ void Test_OS_ObjectIdTransaction(void)
     UtAssert_STUB_COUNT(OS_Lock_Global_Impl, 0);
 
     /* shutdown will prevent transactions */
-    OS_SharedGlobalVars.Initialized  = true;
-    OS_SharedGlobalVars.ShutdownFlag = OS_SHUTDOWN_MAGIC_NUMBER;
+    OS_SharedGlobalVars.GlobalState = OS_SHUTDOWN_MAGIC_NUMBER;
     OSAPI_TEST_FUNCTION_RC(OS_ObjectIdTransactionInit(OS_LOCK_MODE_GLOBAL, OS_OBJECT_TYPE_OS_BINSEM, &token),
                            OS_ERR_INCORRECT_OBJ_STATE);
     UtAssert_UINT32_EQ(token.lock_mode, OS_LOCK_MODE_NONE);
@@ -793,7 +791,7 @@ void Test_OS_ObjectIdTransaction(void)
     UtAssert_STUB_COUNT(OS_Unlock_Global_Impl, 1);
 
     /* other cases for normal operating mode */
-    OS_SharedGlobalVars.ShutdownFlag = 0;
+    OS_SharedGlobalVars.GlobalState = OS_INIT_MAGIC_NUMBER;
     OSAPI_TEST_FUNCTION_RC(OS_ObjectIdTransactionInit(OS_LOCK_MODE_GLOBAL, OS_OBJECT_TYPE_OS_COUNTSEM, &token),
                            OS_SUCCESS);
     UtAssert_UINT32_EQ(token.lock_mode, OS_LOCK_MODE_GLOBAL);
@@ -1081,10 +1079,10 @@ void Osapi_Test_Setup(void)
 
     /*
      * The OS_SharedGlobalVars is also used here, but set the
-     * "Initialized" field to true by default, as this is needed by most tests.
+     * "GlobalState" field to init by default, as this is needed by most tests.
      */
     memset(&OS_SharedGlobalVars, 0, sizeof(OS_SharedGlobalVars));
-    OS_SharedGlobalVars.Initialized = true;
+    OS_SharedGlobalVars.GlobalState = OS_INIT_MAGIC_NUMBER;
 }
 
 /*

src/unit-test-coverage/shared/src/coveragetest-printf.c

@@ -68,13 +68,13 @@ void Test_OS_printf(void)
 
     /* catch case where OS_printf called before init */
     OS_SharedGlobalVars.PrintfConsoleId = OS_OBJECT_ID_UNDEFINED;
-    OS_SharedGlobalVars.Initialized     = false;
+    OS_SharedGlobalVars.GlobalState     = 0;
     OS_printf("UnitTest1");
     UtAssert_True(OS_console_table[0].WritePos == 0, "WritePos (%lu) >= 0",
                   (unsigned long)OS_console_table[0].WritePos);
 
     /* because printf is disabled, the call count should _not_ increase here */
-    OS_SharedGlobalVars.Initialized = true;
+    OS_SharedGlobalVars.GlobalState = OS_INIT_MAGIC_NUMBER;
     OS_printf_disable();
     OS_printf("UnitTest2");
     UtAssert_True(OS_console_table[0].WritePos == 0, "WritePos (%lu) >= 0",