Skip to content

Commit

Permalink
Merge pull request #598 from jphickey/fix-295-apptable-scan
Browse files Browse the repository at this point in the history
Fix #295, Resolve app table scanning race conditions
  • Loading branch information
astrogeco authored Apr 21, 2020
2 parents 5a1c7e8 + bf24986 commit a4a48bd
Show file tree
Hide file tree
Showing 8 changed files with 509 additions and 455 deletions.
462 changes: 211 additions & 251 deletions fsw/cfe-core/src/es/cfe_es_api.c

Large diffs are not rendered by default.

338 changes: 193 additions & 145 deletions fsw/cfe-core/src/es/cfe_es_apps.c

Large diffs are not rendered by default.

40 changes: 23 additions & 17 deletions fsw/cfe-core/src/es/cfe_es_apps.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
*/

/*
** File:
** File:
** cfe_es_apps.h
**
** Purpose:
Expand Down Expand Up @@ -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;

Expand All @@ -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
{
Expand All @@ -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;


Expand All @@ -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;

/*
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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);

Expand Down
6 changes: 6 additions & 0 deletions fsw/cfe-core/src/es/cfe_es_backgroundtask.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
19 changes: 6 additions & 13 deletions fsw/cfe-core/src/es/cfe_es_task.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;


Expand Down Expand Up @@ -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
{
Expand Down
5 changes: 5 additions & 0 deletions fsw/cfe-core/src/es/cfe_es_task.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/*
Expand Down
19 changes: 17 additions & 2 deletions fsw/cfe-core/src/inc/cfe_es_extern_typedefs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

};

/**
Expand Down Expand Up @@ -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
};

/**
Expand Down
75 changes: 48 additions & 27 deletions fsw/cfe-core/unit-test/es_UT.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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;

Expand All @@ -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;

Expand Down Expand Up @@ -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;
Expand All @@ -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.,
Expand Down Expand Up @@ -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",
Expand All @@ -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);
Expand Down

0 comments on commit a4a48bd

Please sign in to comment.