Restart app delete's app prior to missing file check (requirement failure)

[skliper]

Describe the bug
Requirement cES1007.2 - If the original cFE Application file is not found then the cFE shall reject the Command, increment the invalid Command counter, and generate an event message.
Rationale: Can't restart the Application if the original file has been removed. In this case, the Application will continue without a restart.

If you send a restart now with the file remove, the app exits prior to the restart failing.

To Reproduce
Steps to reproduce the behavior:

1. Normal startup (with sample_app)
2. Remove sample_app.so
3. Send ES Restart app for "SAMPLE_APP"
4. Observe error:

1980-012-14:05:46.50026 CFE_ES_RestartApp: Restart Application SAMPLE_APP Initiated
1980-012-14:05:47.00007 CFE_ES_ExitApp: Called with invalid status (1).
1980-012-14:05:47.00008 Application SAMPLE_APP called CFE_ES_ExitApp
EVS Port1 42/1/CFE_ES 14: Exit Application SAMPLE_APP on Error Completed.

Expected behavior
Behavior matches description, likely due an update to move behavior in rationale to requirement. Should just check for file existing prior to delete of app.

Code snips

cFE/fsw/cfe-core/src/es/cfe_es_api.c

Lines 250 to 252 in 3e60d95

	CFE_ES_Global.AppTable[AppID].ControlReq.AppControlRequest = CFE_ES_RunStatus_SYS_RESTART;
	CFE_ES_SetAppState(AppID, CFE_ES_AppState_WAITING);
	CFE_ES_Global.AppTable[AppID].ControlReq.AppTimer = CFE_PLATFORM_ES_APP_KILL_TIMEOUT;

cFE/fsw/cfe-core/src/es/cfe_es_apps.c

Lines 1079 to 1095 in 3e60d95

	case CFE_ES_RunStatus_SYS_RESTART:
	/*
	** Kill the app
	*/
	Status = CFE_ES_CleanUpApp(AppID);
	if ( Status == CFE_SUCCESS )
	{
	/*
	** And start it back up again
	*/
	Status = CFE_ES_AppCreate(&NewAppId, (char *)AppStartParams.FileName,
	(char *)AppStartParams.EntryPoint,
	(char *)AppStartParams.Name,
	AppStartParams.Priority,
	AppStartParams.StackSize,
	AppStartParams.ExceptionAction);

System observed on:

• cFS Dev server
• OS: Ubuntu 16.04
• Versions: current dev cFE 6.7.3

Additional context
None.

Reporter Info
Jacob Hageman - NASA/GSFC

[jphickey]

Does this really need to be a requirement? Seems like a superfluous test to me.

I recall this being discussed (or at least something similar) years ago, and this doesn't seem like a valid requirement. This is a "restart", so the app was loaded once. The only way it could not exist now would be if it was deleted between the time the app was first started and now, possibly due to a failed upload or something. In other words, some type of operator intervention would have had to break it first.

But that is just one obvious failure mode. Even with a "file exists" check there are plenty of other issues that could prevent this from working, which are not as predictable. Maybe the file got corrupted, so it exists but it is un-loadable. Maybe there isn't enough memory to load the new app, or the old one couldn't be unloaded because there was some dependency still bound to it.

It boils down to operator error. The system can't prevent you from breaking it, nor should it try to be smarter than the ground system operators. If someone sends a bad command, so be it.

[skliper]

I have no objection to making the requirement match the code (remove the "Application will continue" part of the rational). @acudmore or @jwilmot ?

[skliper]

I'd basically add something like the following in the rationale "The current app is removed prior to attempt to load, so any issues with the restart (missing file, corrupted file, out of memory, etc) will leave the system with the app removed."

[skliper]

See cES1007.2 updates in #509

[skliper]

Requirement update done as part of #509, closing as duplicate.