Merge pull request #956 from nasa/integration-candidate

osal Integration candidate: 2021-04-13

README.md

@@ -11,7 +11,15 @@ The autogenerated OSAL user's guide can be viewed at <https://github.com/nasa/cF
 
 ## Version History
 
-
+### Development Build: v5.1.0-rc1+dev387
+
+- Replaces the separate "Initialized" and "Shutdown" flags with a single state flag. Creates a global single source of truth for the OSAL state. This enables users to run tests and OS_API_Init() multiple times without a reboot in the middle to reset the state.
+  - Multiple invocations of OS_API_Init() are allowed - subsequent calls can be ignored
+  - Deleting of any internal objects that did get created if OS_API_Init() fails (this 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.
+- Adds OS_API_Teardown to complement OS_API_Init. This cleans up all OSAL resources ideally leaving the system in a state where `OS_API_Init()` may be invoked again.
+- Reworks the shell unit test which was probably not working. Note this requires modifying the osal config to enable shell, otherwise test is skipped.
+- See <https://github.com/nasa/osal/pull/956> and <https://github.com/nasa/cFS/pull/242>
 
 ### Development Build: v5.1.0-rc1+dev378
 

src/os/inc/osapi-common.h

@@ -140,6 +140,33 @@ void OS_Application_Run(void);
  */
 int32 OS_API_Init(void);
 
+/*-------------------------------------------------------------------------------------*/
+/**
+ * @brief Teardown/de-initialization of OSAL API
+ *
+ * This is the inverse of OS_API_Init().  It will release all OS resources and
+ * return the system to a state similar to what it was prior to invoking
+ * OS_API_Init() initially.
+ *
+ * Normally for embedded applications, the OSAL is initialized after boot and will remain
+ * initialized in memory until the processor is rebooted.  However for testing and
+ * developement purposes, it is potentially useful to reset back to initial conditions.
+ *
+ * For testing purposes, this API is designed/intended to be compatible with the
+ * UtTest_AddTeardown() routine provided by the UT-Assert subsystem.
+ *
+ * @note This is a "best-effort" routine and it may not always be possible/guaranteed
+ * to recover all resources, particularly in the case of off-nominal conditions, or if
+ * a resource is used outside of OSAL.
+ *
+ * For example, while this will attempt to unload all dynamically-loaded modules, doing
+ * so may not be possible and/or may induce undefined behavior if resources are in use by
+ * tasks/functions outside of OSAL.
+ *
+ * @return None
+ */
+void OS_API_Teardown(void);
+
 /*-------------------------------------------------------------------------------------*/
 /**
  * @brief Background thread implementation - waits forever for events to occur.

src/os/inc/osapi-version.h

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

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,9 +231,43 @@ 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 */
 
+/*----------------------------------------------------------------
+ *
+ * Function: OS_API_Teardown
+ *
+ *  Purpose: Implemented per public OSAL API
+ *           See description in API and header file for detail
+ *
+ *-----------------------------------------------------------------*/
+void OS_API_Teardown(void)
+{
+    /*
+     * This should delete any remaining user-created objects/tasks
+     */
+    OS_DeleteAllObjects();
+
+    /*
+     * This should cause the "internal" objects (e.g. console utility task)
+     * to exit, and will prevent any new objects from being created.
+     */
+    OS_ApplicationShutdown(true);
+}
+
 /*----------------------------------------------------------------
  *
  * Function: OS_RegisterEventHandler
@@ -359,7 +408,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 +426,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/tests/bin-sem-flush-test/bin-sem-flush-test.c

@@ -177,6 +177,9 @@ void UtTest_Setup(void)
         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
      */

src/tests/bin-sem-test/bin-sem-test.c

@@ -186,6 +186,9 @@ void UtTest_Setup(void)
         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
      */

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

@@ -181,6 +181,9 @@ void UtTest_Setup(void)
         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
      */

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

@@ -151,6 +151,9 @@ void UtTest_Setup(void)
         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
      */

src/tests/file-api-test/file-api-test.c

@@ -57,6 +57,9 @@ void UtTest_Setup(void)
         UtAssert_Abort("OS_API_Init() failed");
     }
 
+    /* the test should call OS_API_Teardown() before exiting */
+    UtTest_AddTeardown(OS_API_Teardown, "Cleanup");
+
     /*
      * This test case requires a fixed virtual dir for one test case.
      * Just map /test to a dir of the same name, relative to current dir.

src/tests/file-sys-add-fixed-map-api-test/file-sys-add-fixed-map-api-test.c

@@ -96,6 +96,9 @@ void UtTest_Setup(void)
         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
      */

src/tests/idmap-api-test/idmap-api-test.c

@@ -348,6 +348,9 @@ void UtTest_Setup(void)
         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
      */

src/tests/mutex-test/mutex-test.c

@@ -217,6 +217,9 @@ void UtTest_Setup(void)
         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
      */

src/tests/network-api-test/network-api-test.c

@@ -658,6 +658,9 @@ void UtTest_Setup(void)
         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
      */

src/tests/osal-core-test/osal-core-test.c

@@ -96,6 +96,9 @@ void UtTest_Setup(void)
         UtAssert_Abort("OS_API_Init() failed");
     }
 
+    /* the test should call OS_API_Teardown() before exiting */
+    UtTest_AddTeardown(OS_API_Teardown, "Cleanup");
+
     UtTest_Add(TestTasks, NULL, NULL, "TASK");
     UtTest_Add(TestQueues, NULL, NULL, "MSGQ");
     UtTest_Add(TestBinaries, NULL, NULL, "BSEM");