CFE_ES_RunLoop increments the main task ExecutionCounter rather than the task that called it

[jphickey]

Is your feature request related to a problem? Please describe.

Inside the CFE_ES_RunLoop() function itself, it looks up the MainTaskId of the caller and only increments that execution counter, not the execution counter of its own task record. This means no matter what child task actually calls this function, it implements the execution counter of the main task only.

Note that if the real main task is doing something else and also increments its own task counter, this is a race condition.

Describe the solution you'd like
I suggest one of the following:

1. Just have CFE_ES_RunLoop() invoke CFE_ES_IncrementTaskCounter() to increment the counter for the task from which it was called. So if it gets called from a child task, then that child task gets incremented, not the main task. This is at least straightforward/consistent and avoids the race condition.
2. Maintain a separate "app" exec counter which is incremented by CFE_ES_RunLoop (only), and use the CFE_ES_IncrementTaskCounter to account for other regular task activity.

Additional context
Noticed this when fixing #480 and it seemed rather odd/incorrect to be storing the ExecutionCounter where it is. This causes the code to jump to other entries in the table for the sole purpose of reading/updating this value, when it already had the correct app record to start.

Requester Info
Joseph Hickey, Vantage Systems, Inc.

[ejtimmon]

FWIW I recently got some questions from app developers about this (wanting to know whether the exec counter should be incremented in child tasks) - the change suggested would clarify that. I did get the sense from those users that they wanted a way to track the execution of child tasks in addition to the main task.

[jphickey]

@ejtimmon After initially writing this issue I then rediscovered that there is a function called CFE_ES_IncrementTaskCounter which increments the counter for the task context which invoked it.

So yes, there is actually the notion of a child task counter....

But then it is still unclear to me why the CFE_ES_RunLoop function would increment the counter for the main task of the app, rather than the task that actually invoked the function - this doesn't make sense, because if the main task also calls CFE_ES_IncrementTaskCounter() like child tasks do then it will get double incremented, or if a child task calls CFE_ES_RunLoop() then that activity will actually get accounted for in the main task (which is just plain wrong).

I suggest one of the following:

1. Just have CFE_ES_RunLoop() invoke CFE_ES_IncrementTaskCounter() to increment the counter for the task from which it was called. So if it gets called from a child task, then that child task gets incremented, not the main task.
2. Maintain a separate "app" exec counter which is incremented by CFE_ES_RunLoop (only), and use the CFE_ES_IncrementTaskCounter for regular task activity accounting.

At the very minimum, it is probably at least worth a note in the documentation of CFE_ES_RunLoop that it is incrementing a task counter (whichever one it is) because the documentation doesn't seem to mention it at all, and this could certainly confuse someone (it confused me...).

Probably worth a quick discussion to see what others think...

[ejtimmon]

Whenever a decision is made, we should also add guidance to the App Developer's Guide.

[astrogeco]

CCB 2020-03-25: Discussed, will have telemetry impacts based on app vs task counter. Will also affect Health and Safety's behavior. Behavior needs to be documented in users guide, document that "run loop" should be called from main task.

[skliper]

Is this needed for Caelum? Should we touch on again in CCB?

[jphickey]

Note that previous changes in this area did in fact change CFE_ES_RunLoop to call the CFE_ES_IncrementTaskCounter to increment the counter. Simply put - it has to be this way because having any task increment a counter other than its own (i.e. the old way) would be a race condition. So this is not only simpler - just one common function to increment exec counter - it is also safer.

If guidelines/templates were followed this is no effective change because CFE_ES_RunLoop is called from the main task - so its the same counter. But if guidelines/templates were not followed and this is called from a child task then it will increment the counter of the child task, not the main task.

[jphickey]

If anything we should just update the documentation of CFE_ES_RunLoop to indicate this -- that it increments the task execution counter of the task that called it, and is expected to be invoked from the "main" task of a CFE application, as part of that app's main loop.

[skliper]

Implementation change addressed by #598. Remaining work here is to update documentation.