Merge pull request #598 from jphickey/fix-295-apptable-scan

Fix #295, Resolve app table scanning race conditions

fsw/cfe-core/src/es/cfe_es_apps.h

@@ -19,7 +19,7 @@
 */
 
 /*
-**  File: 
+**  File:
 **  cfe_es_apps.h
 **
 **  Purpose:
@@ -58,8 +58,8 @@
 */
 typedef struct
 {
-   uint32     AppControlRequest;              /* What the App should be doing next */
-   int32      AppTimer;                       /* Countdown timer for killing an app */
+    uint32     AppControlRequest;   /* What the App should be doing next */
+    int32      AppTimerMsec;        /* Countdown timer for killing an app, in milliseconds */
 
 } CFE_ES_ControlReq_t;
 
@@ -80,13 +80,13 @@ typedef struct
 
   uint16                ExceptionAction;
   uint16                Priority;
-   
+
 } CFE_ES_AppStartParams_t;
 
 /*
 ** CFE_ES_MainTaskInfo_t is a structure of information about the main
 ** task and child tasks in a cFE application. This structure is just used in the
-** cFE_ES_AppRecord_t structure. 
+** cFE_ES_AppRecord_t structure.
 */
 typedef struct
 {
@@ -106,7 +106,7 @@ typedef struct
    CFE_ES_AppStartParams_t StartParams;                 /* The start parameters for an App */
    CFE_ES_ControlReq_t     ControlReq;                  /* The Control Request Record for External cFE Apps */
    CFE_ES_MainTaskInfo_t   TaskInfo;                    /* Information about the Tasks */
-      
+
 } CFE_ES_AppRecord_t;
 
 
@@ -121,8 +121,8 @@ typedef struct
    uint32    TaskId;                          /* Task ID */
    uint32    ExecutionCounter;                /* The execution counter for the Child task */
    char      TaskName[OS_MAX_API_NAME];       /* Task Name */
-   
-   
+
+
 } CFE_ES_TaskRecord_t;
 
 /*
@@ -135,6 +135,19 @@ typedef struct
    char      LibName[OS_MAX_API_NAME];        /* Library Name */
 } CFE_ES_LibRecord_t;
 
+/*
+** CFE_ES_AppTableScanState_t is an internal structure used to keep state of
+** the background app table scan/cleanup process
+*/
+typedef struct
+{
+    uint32 PendingAppStateChanges;
+    uint32 BackgroundScanTimer;
+    uint8  LastScanCommandCount;
+} CFE_ES_AppTableScanState_t;
+
+
+
 /*****************************************************************************/
 /*
 ** Function prototypes
@@ -150,13 +163,6 @@ void  CFE_ES_StartApplications(uint32 ResetType, const char *StartFilePath );
 */
 int32 CFE_ES_ParseFileEntry(const char **TokenList, uint32 NumTokens);
 
-/*
- * Internal function to set the state of an app
- * All state changes should go through this function rather than directly writing to the control block
- */
-void CFE_ES_SetAppState(uint32 AppID, uint32 TargetState);
-
-
 /*
 ** Internal function to create/start a new cFE app
 ** based on the parameters passed in
@@ -190,7 +196,7 @@ int32 CFE_ES_AppDumpAllInfo(void);
 /*
 ** Scan the Application Table for actions to take
 */
-void CFE_ES_ScanAppTable(void);
+bool CFE_ES_RunAppTableScan(uint32 ElapsedTime, void *Arg);
 
 /*
 ** Perform the requested control action for an application
@@ -208,7 +214,7 @@ int32 CFE_ES_CleanUpApp(uint32 AppId);
 int32 CFE_ES_CleanupTaskResources(uint32 TaskId);
 
 /*
-** Debug function to print out resource utilization 
+** Debug function to print out resource utilization
 */
 int32 CFE_ES_ListResourcesDebug(void);
 

fsw/cfe-core/src/es/cfe_es_backgroundtask.c

@@ -71,6 +71,12 @@ typedef struct
  */
 const CFE_ES_BackgroundJobEntry_t CFE_ES_BACKGROUND_JOB_TABLE[] =
 {
+        {   /* ES app table background scan */
+                .RunFunc = CFE_ES_RunAppTableScan,
+                .JobArg = &CFE_ES_TaskData.BackgroundAppScanState,
+                .ActivePeriod = CFE_PLATFORM_ES_APP_SCAN_RATE / 4,
+                .IdlePeriod = CFE_PLATFORM_ES_APP_SCAN_RATE
+        },
         {   /* Performance Log Data Dump to file */
                 .RunFunc = CFE_ES_RunPerfLogDump,
                 .JobArg = &CFE_ES_TaskData.BackgroundPerfDumpState,

fsw/cfe-core/src/es/cfe_es_task.c

@@ -80,7 +80,6 @@ CFE_ES_TaskData_t CFE_ES_TaskData;
 void CFE_ES_TaskMain(void)
 {
     int32   Status;
-    uint32  TimeOut = CFE_PLATFORM_ES_APP_SCAN_RATE;
     uint32  AppRunStatus = CFE_ES_RunStatus_APP_RUN;
 
 
@@ -139,31 +138,25 @@ void CFE_ES_TaskMain(void)
         */
         Status = CFE_SB_RcvMsg(&CFE_ES_TaskData.MsgPtr,
                                   CFE_ES_TaskData.CmdPipe,
-                                  TimeOut);
+                                  CFE_SB_PEND_FOREVER);
 
         /*
         ** Performance Time Stamp Entry
         */
         CFE_ES_PerfLogEntry(CFE_MISSION_ES_MAIN_PERF_ID);
 
-        /*
-        ** Scan the App table for Application Deletion requests
-        */
-        if ( Status == CFE_SB_TIME_OUT )
-        {
-           CFE_ES_ScanAppTable();
-        }
-        else if (Status == CFE_SUCCESS)
+        if (Status == CFE_SUCCESS)
         {
            /*
            ** Process Software Bus message.
            */
            CFE_ES_TaskPipe(CFE_ES_TaskData.MsgPtr);
 
            /*
-           ** Scan the App Table for changes after processing a command
-           */
-           CFE_ES_ScanAppTable();
+            * Wake up the background task, which includes the
+            * scanning of the ES app table for entries that may need cleanup
+            */
+           CFE_ES_BackgroundWakeup();
         }
         else
         {

fsw/cfe-core/src/es/cfe_es_task.h

@@ -113,6 +113,11 @@ typedef struct
    */
   CFE_ES_PerfDumpGlobal_t    BackgroundPerfDumpState;
 
+  /*
+   * Persistent state data associated with background app table scans
+   */
+  CFE_ES_AppTableScanState_t BackgroundAppScanState;
+
 } CFE_ES_TaskData_t;
 
 /*

fsw/cfe-core/src/inc/cfe_es_extern_typedefs.h

@@ -116,6 +116,10 @@ typedef uint8                                            CFE_ES_AppType_Enum_t;
  */
 enum CFE_ES_RunStatus
 {
+    /**
+     * @brief Reserved value, should not be used
+     */
+    CFE_ES_RunStatus_UNDEFINED                        = 0,
 
    /**
     * @brief Indicates that the Application should continue to run
@@ -160,7 +164,13 @@ enum CFE_ES_RunStatus
    /**
     * @brief Indicates that the Core Application had a runtime failure
     */
-   CFE_ES_RunStatus_CORE_APP_RUNTIME_ERROR            = 9
+   CFE_ES_RunStatus_CORE_APP_RUNTIME_ERROR            = 9,
+
+   /**
+    * @brief Reserved value, marker for the maximum state
+    */
+   CFE_ES_RunStatus_MAX
+
 };
 
 /**
@@ -211,7 +221,12 @@ enum CFE_ES_SystemState
    /**
     * @brief reserved for future use, all apps would be STOPPED
     */
-   CFE_ES_SystemState_SHUTDOWN                        = 6
+   CFE_ES_SystemState_SHUTDOWN                        = 6,
+
+   /**
+    * @brief Reserved value, marker for the maximum state
+    */
+   CFE_ES_SystemState_MAX
 };
 
 /**

fsw/cfe-core/unit-test/es_UT.c

@@ -1324,13 +1324,14 @@ void TestApps(void)
     CFE_ES_Global.AppTable[Id].Type = CFE_ES_AppType_EXTERNAL;
     CFE_ES_Global.AppTable[Id].AppState = CFE_ES_AppState_WAITING;
     CFE_ES_Global.AppTable[Id].ControlReq.AppControlRequest = CFE_ES_RunStatus_APP_RUN;
-    CFE_ES_Global.AppTable[Id].ControlReq.AppTimer = 0;
-    CFE_ES_ScanAppTable();
+    CFE_ES_Global.AppTable[Id].ControlReq.AppTimerMsec = 0;
+    memset(&CFE_ES_TaskData.BackgroundAppScanState, 0, sizeof(CFE_ES_TaskData.BackgroundAppScanState));
+    CFE_ES_RunAppTableScan(0, &CFE_ES_TaskData.BackgroundAppScanState);
     UT_Report(__FILE__, __LINE__,
               UT_EventIsInHistory(CFE_ES_PCR_ERR2_EID) &&
-              CFE_ES_Global.AppTable[Id].ControlReq.AppTimer == 0 &&
+              CFE_ES_Global.AppTable[Id].ControlReq.AppTimerMsec == 0 &&
               CFE_ES_Global.AppTable[Id].ControlReq.AppControlRequest == CFE_ES_RunStatus_SYS_DELETE,
-              "CFE_ES_ScanAppTable",
+              "CFE_ES_RunAppTableScan",
               "Waiting; process control request");
 
     /* Test scanning and acting on the application table where the timer
@@ -1342,12 +1343,12 @@ void TestApps(void)
     CFE_ES_Global.AppTable[Id].Type = CFE_ES_AppType_EXTERNAL;
     CFE_ES_Global.AppTable[Id].AppState = CFE_ES_AppState_WAITING;
     CFE_ES_Global.AppTable[Id].ControlReq.AppControlRequest = CFE_ES_RunStatus_APP_EXIT;
-    CFE_ES_Global.AppTable[Id].ControlReq.AppTimer = 5;
-    CFE_ES_ScanAppTable();
+    CFE_ES_Global.AppTable[Id].ControlReq.AppTimerMsec = 5000;
+    CFE_ES_RunAppTableScan(1000, &CFE_ES_TaskData.BackgroundAppScanState);
     UT_Report(__FILE__, __LINE__,
-              CFE_ES_Global.AppTable[Id].ControlReq.AppTimer == 4 &&
+              CFE_ES_Global.AppTable[Id].ControlReq.AppTimerMsec == 4000 &&
               CFE_ES_Global.AppTable[Id].ControlReq.AppControlRequest == CFE_ES_RunStatus_APP_EXIT,
-              "CFE_ES_ScanAppTable",
+              "CFE_ES_RunAppTableScan",
               "Decrement timer");
 
     /* Test scanning and acting on the application table where the application
@@ -1359,13 +1360,13 @@ void TestApps(void)
     CFE_ES_Global.AppTable[Id].Type = CFE_ES_AppType_EXTERNAL;
     CFE_ES_Global.AppTable[Id].AppState = CFE_ES_AppState_STOPPED;
     CFE_ES_Global.AppTable[Id].ControlReq.AppControlRequest = CFE_ES_RunStatus_APP_RUN;
-    CFE_ES_Global.AppTable[Id].ControlReq.AppTimer = 0;
-    CFE_ES_ScanAppTable();
+    CFE_ES_Global.AppTable[Id].ControlReq.AppTimerMsec = 0;
+    CFE_ES_RunAppTableScan(0, &CFE_ES_TaskData.BackgroundAppScanState);
     UT_Report(__FILE__, __LINE__,
               UT_EventIsInHistory(CFE_ES_PCR_ERR2_EID) &&
               CFE_ES_Global.AppTable[Id].ControlReq.AppControlRequest == CFE_ES_RunStatus_SYS_DELETE &&
-              CFE_ES_Global.AppTable[Id].ControlReq.AppTimer == 0,
-              "CFE_ES_ScanAppTable",
+              CFE_ES_Global.AppTable[Id].ControlReq.AppTimerMsec == 0,
+              "CFE_ES_RunAppTableScan",
               "Stopped; process control request");
 
     /* Test scanning and acting on the application table where the application
@@ -1376,13 +1377,13 @@ void TestApps(void)
     Id = ES_UT_OSALID_TO_ARRAYIDX(TestObjId);
     CFE_ES_Global.AppTable[Id].Type = CFE_ES_AppType_EXTERNAL;
     CFE_ES_Global.AppTable[Id].AppState = CFE_ES_AppState_EARLY_INIT;
-    CFE_ES_Global.AppTable[Id].ControlReq.AppTimer = 5;
+    CFE_ES_Global.AppTable[Id].ControlReq.AppTimerMsec = 5000;
 
-    CFE_ES_ScanAppTable();
+    CFE_ES_RunAppTableScan(0, &CFE_ES_TaskData.BackgroundAppScanState);
     UT_Report(__FILE__, __LINE__,
               UT_GetNumEventsSent() == 0 &&
-              CFE_ES_Global.AppTable[Id].ControlReq.AppTimer == 5,
-              "CFE_ES_ScanAppTable",
+              CFE_ES_Global.AppTable[Id].ControlReq.AppTimerMsec == 5000,
+              "CFE_ES_RunAppTableScan",
               "Initializing; process control request");
 
    /* Test a control action request on an application with an
@@ -1968,12 +1969,12 @@ void TestApps(void)
     Id = ES_UT_OSALID_TO_ARRAYIDX(TestObjId);
     CFE_ES_Global.AppTable[Id].Type = CFE_ES_AppType_CORE;
     CFE_ES_Global.AppTable[Id].AppState = CFE_ES_AppState_WAITING;
-    CFE_ES_Global.AppTable[Id].ControlReq.AppTimer = 0;
-    CFE_ES_ScanAppTable();
+    CFE_ES_Global.AppTable[Id].ControlReq.AppTimerMsec = 0;
+    CFE_ES_RunAppTableScan(0, &CFE_ES_TaskData.BackgroundAppScanState);
     UT_Report(__FILE__, __LINE__,
               UT_GetNumEventsSent() == 0 &&
-              CFE_ES_Global.AppTable[Id].ControlReq.AppTimer == 0,
-              "CFE_ES_ScanAppTable",
+              CFE_ES_Global.AppTable[Id].ControlReq.AppTimerMsec == 0,
+              "CFE_ES_RunAppTableScan",
               "Waiting; process control request");
     CFE_ES_Global.TaskTable[Id].RecordUsed = false;
 
@@ -1985,12 +1986,12 @@ void TestApps(void)
     Id = ES_UT_OSALID_TO_ARRAYIDX(TestObjId);
     CFE_ES_Global.AppTable[Id].Type = CFE_ES_AppType_EXTERNAL;
     CFE_ES_Global.AppTable[Id].AppState = CFE_ES_AppState_RUNNING;
-    CFE_ES_Global.AppTable[Id].ControlReq.AppTimer = 0;
-    CFE_ES_ScanAppTable();
+    CFE_ES_Global.AppTable[Id].ControlReq.AppTimerMsec = 0;
+    CFE_ES_RunAppTableScan(0, &CFE_ES_TaskData.BackgroundAppScanState);
     UT_Report(__FILE__, __LINE__,
               UT_GetNumEventsSent() == 0 &&
-              CFE_ES_Global.AppTable[Id].ControlReq.AppTimer == 0,
-              "CFE_ES_ScanAppTable",
+              CFE_ES_Global.AppTable[Id].ControlReq.AppTimerMsec == 0,
+              "CFE_ES_RunAppTableScan",
               "Running; process control request");
     CFE_ES_Global.TaskTable[Id].RecordUsed = false;
 
@@ -4467,12 +4468,14 @@ void TestAPI(void)
 
     /* Test exiting an app with an exit error */
     /* Note - this exit code of 1000 is invalid, which causes
-     * an extra message to be logged in syslog about this */
+     * an extra message to be logged in syslog about this.  This
+     * should also be stored in the AppControlRequest as APP_ERROR. */
     ES_ResetUnitTest();
     OS_TaskCreate(&TestObjId, "UT", NULL, NULL, 0, 0, 0);
     Id = ES_UT_OSALID_TO_ARRAYIDX(TestObjId);
     CFE_ES_Global.TaskTable[Id].AppId = Id;
     CFE_ES_Global.TaskTable[Id].RecordUsed = true;
+    CFE_ES_Global.AppTable[Id].ControlReq.AppControlRequest = CFE_ES_RunStatus_APP_RUN;
     CFE_ES_Global.AppTable[Id].Type = CFE_ES_AppType_EXTERNAL;
     CFE_ES_Global.AppTable[Id].AppState = CFE_ES_AppState_STOPPED;
     CFE_ES_Global.AppTable[Id].Type = CFE_ES_AppType_CORE;
@@ -4482,6 +4485,10 @@ void TestAPI(void)
                   UT_GetStubCount(UT_KEY(OS_printf)) == 2,
               "CFE_ES_ExitApp",
               "Application exit error");
+    UtAssert_True(CFE_ES_Global.AppTable[Id].ControlReq.AppControlRequest == CFE_ES_RunStatus_APP_ERROR,
+            "CFE_ES_ExitApp - AppControlRequest (%u) == CFE_ES_RunStatus_APP_ERROR (%u)",
+            (unsigned int)CFE_ES_Global.AppTable[Id].ControlReq.AppControlRequest,
+            (unsigned int)CFE_ES_RunStatus_APP_ERROR);
 
 #if 0
     /* Can't cover this path since it contains a while(1) (i.e.,
@@ -4548,9 +4555,9 @@ void TestAPI(void)
     CFE_ES_Global.AppTable[Id].AppState = CFE_ES_AppState_RUNNING;
     CFE_ES_Global.TaskTable[Id].RecordUsed = false;
     CFE_ES_Global.TaskTable[Id].AppId = Id;
-    RunStatus = CFE_ES_RunStatus_APP_EXIT;
+    RunStatus = CFE_ES_RunStatus_APP_RUN;
     CFE_ES_Global.AppTable[Id].ControlReq.AppControlRequest =
-        CFE_ES_RunStatus_APP_EXIT;
+        CFE_ES_RunStatus_APP_RUN;
     UT_Report(__FILE__, __LINE__,
               CFE_ES_RunLoop(&RunStatus) == false,
               "CFE_ES_RunLoop",
@@ -4571,6 +4578,20 @@ void TestAPI(void)
               "CFE_ES_RunLoop",
               "Invalid run status");
 
+    /* Test run loop with a NULL run status */
+    ES_ResetUnitTest();
+    OS_TaskCreate(&TestObjId, "UT", NULL, NULL, 0, 0, 0);
+    Id = ES_UT_OSALID_TO_ARRAYIDX(TestObjId);
+    CFE_ES_Global.AppTable[Id].AppState = CFE_ES_AppState_RUNNING;
+    CFE_ES_Global.TaskTable[Id].RecordUsed = true;
+    CFE_ES_Global.TaskTable[Id].AppId = Id;
+    CFE_ES_Global.AppTable[Id].ControlReq.AppControlRequest =
+            CFE_ES_RunStatus_APP_RUN;
+    UT_Report(__FILE__, __LINE__,
+              CFE_ES_RunLoop(NULL),
+              "CFE_ES_RunLoop",
+              "Nominal, NULL output pointer");
+
     /* Test run loop with startup sync code */
     ES_ResetUnitTest();
     OS_TaskCreate(&TestObjId, "UT", NULL, NULL, 0, 0, 0);