Fix #1033, remove false child tasks statement. #1193
Conversation
| - If the Main Task of an Application is stopped, either through | ||
| detection of an exception or via command, all Child Tasks are also | ||
| stopped. |
There was a problem hiding this comment.
Is this comment valid? Maybe there should be a more general comment related to the implementations? @jphickey - recommendations?
There was a problem hiding this comment.
Its not totally invalid, it just glosses over the details....
If an exception occurs then only the specific task that triggered the exception is immediately stopped - Other tasks will keep running.
However, such an event will put the ES app record into an alternate exception state - and after a time delay (typically ~5 seconds) the ES will clean up the app in its background job. As part of this clean up, all child tasks and the main task will be removed if they have not already exited.
So eventually all tasks will be stopped, but it is not quite as that statement implies (the wording suggests/implies this happens at the same time, but it does not happen at the same time).
There was a problem hiding this comment.
I think it's the generic language that bugs me the most since there's subtilties that don't seem to be sufficiently addressed by the original comment that could lead to incorrect assumptions. Maybe it would help to start with "When ES stops or cleans up an application...(add your details here)"
There was a problem hiding this comment.
If the main task is stopped via command, do the child tasks stop at the same time or do they still sit around until the ES clean up happens?
There was a problem hiding this comment.
@zanzaben - there really is no "main task stopped by command" - the stop command being referred to here works on an App basis, not a task basis, and it's really a voluntary request, not a forced administrative action (at least at first).
When this "stop" command is processed, it posts a request for the app to voluntarily stop itself. The next time the main task calls CFE_ES_RunLoop() this function will return false (thereby causing its while loop to exit) and then the app should do an orderly shutdown, closing any resources it was holding and removing its child tasks, and then exit. This "cooperative" approach is the preferred way stop an app.
However, for apps that are not cooperative, there is a backup plan: If it doesn't voluntarily shut itself down within the ES timeout then ES will forcibly kill all the tasks associated with the app, and also forcibly free all the other resources that CFE/OSAL is responsible for tracking that appear to be "owned" by that app. This isn't ideal; it doesn't get everything (it can't) and there is no way to know if we are leaving something in an indeterminate state due to forcibly killing the tasks.
Also - in all cases child tasks continue running until this happens (either voluntary shutdown or delete by ES). It is not synchronous/same time in any case.
There was a problem hiding this comment.
IMO - if ES ever has to forcibly clean up an app in the "unclean" (back-up) manner I described above - it implies an indeterminate state of the overall system. There could be e.g. locks left in a wrong state, and there is no way to know for sure the way things were left. The operator really should do a processor reset to bring it back to a fully known/stable state again.
zanzaben
force-pushed
the
fix1033_Update_App_Dev_Guide
branch
from
e2963ab
to
bedca93
Compare
zanzaben
added
the
CCB:Ready
astrogeco
added
IC:2021-03-23
and removed
CCB:Ready
nasa/cFE#1218 - Adds a local definition of `SOFTWARE_BIG/LITTLE_BIT_ORDER` directly inside `cfe_endian.h` to provide a compatible symbol for apps that still require this. This allows CFE to build and run successfully when OSAL stops providing this in `common_types.h`. nasa/cFE#1193 - Removes incorrect statements from Application Developers Guide nasa/cFE#1235 - Fixes truncation handling on vsnprintf error by adding a cast to avoid implicit conversion nasa/cFE#1220 - Clarify the documentation on SB MsgId regarding requirements for command and telemetry messages nasa/cFE#1230 - Avoids undefined behavior and resolves static analysis warnings by casting `isspace` input to `unsigned char`. nasa/cFE#1231 - Updates message module and msgid v1, `CFE_MSG_SetMsgId`, to use mask instead of cast to alter value. Resolves static analysis warning. nasa/cFE#1232 - Updates `CFE_ES_FileWriteByteCntErr` to report status, not a `size_t` actual since `OS_write` returns `int32`. Use `int16` for local type from `CFE_TBL_FindTableInRegistry` since it's an index, not a status. nasa/cFE#1228 - Replaces `<>` with `"` in local `#include`s nasa/cFE#1227 - Adds `CONTRIBUING.md` that links to the main cFS contributing guide.
nasa/cFE#1243 v6.8.0-rc1+dev436 nasa/cFE#1225, Add coverage test fix nasa/cFE#1218, Adds a local definition of `SOFTWARE_BIG/LITTLE_BIT_ORDER` directly inside `cfe_endian.h` to provide a compatible symbol for apps that still require this. This allows CFE to build and run successfully when OSAL stops providing this in `common_types.h`. nasa/cFE#1193, Removes incorrect statements from Application Developers Guide nasa/cFE#1235, Fixes truncation handling on vsnprintf error by adding a cast to avoid implicit conversion nasa/cFE#1220, Clarify the documentation on SB MsgId regarding requirements for command and telemetry messages nasa/cFE#1230, Avoids undefined behavior and resolves static analysis warnings by casting `isspace` input to `unsigned char`. nasa/cFE#1231, Updates message module and msgid v1, `CFE_MSG_SetMsgId`, to use mask instead of cast to alter value. Resolves static analysis warning. nasa/cFE#1232, Updates `CFE_ES_FileWriteByteCntErr` to report status, not a `size_t` actual since `OS_write` returns `int32`. Use `int16` for local type from `CFE_TBL_FindTableInRegistry` since it's an index, not a status. nasa/cFE#1228, Replaces `<>` with `"` in local `#include`s nasa/cFE#1227, Adds `CONTRIBUING.md` that links to the main cFS contributing guide. nasa/PSP#273 v1.5.0-rc1+dev90 nasa/PSP#264, modular psp implementation nasa/PSP#272, Use quotes for local includes nasa/PSP#271, Add Contributing Guide nasa/osal#917 v5.1.0-rc1+dev347 nasa/osal#890, Move copyblock size to a #define and add comments nasa/osal#891, Removed rogue while loop nasa/osal#892, Scripted replacement for #include <os and #include <OSC_ matches of < and > with " nasa/osal#893, Consolidates the duplicated switch in OS_SocketOpen_Impl nasa/osal#894, Add `const` to input pointers nasa/osal#895, Removed network prototypes defined in osapi_sockets.h that are also in osapi_network.h nasa/osal#896, Removes NULL redefine from common_types.h nasa/osal#912, Add Contributing Guide nasa/osal#914, Utilize UTASSERT_CASETYPE_NA to report OS_ERR_NOT_IMPLEMENTED nasa/osal#898, correct interval calculation in DoSelect nasa/osal#909, comments describing select after connect nasa/osal#902, add check for EAGAIN in addition to EINTR nasa/osal#908, compile time assert for sockaddr size nasa/osal#910, check/report fcntl status nasa/osal#897, Add assert for FD_SET_SIZE in relation to OSAL_set nasa/osal#903, better error translation for ESPIPE errno nasa/osal#840, add shell functional test nasa/osal#901, rename OS_U32ValueWrapper_t nasa/osal#900, break up logic in return statement nasa/osal#906, return moduleInfoGet error nasa/osal#907, report timer_gettime error nasa/osal#899, remove unreachable test nasa/osal#905, make module comment same as other services nasa/osal#920 to fix test error check index inside fdset conversions nasa/osal#922, make non-selectable FD an error nasa/sample_app#137 v1.2.0-rc1+dev54 nasa/sample_app#134, Convert from <> to " for local includes nasa/sample_app#136, Added a contributing guide that links to the main cFS contributing guide. nasa/sample_app#132, Add context to the values for MsgIDs nasa/sample_lib#55 v1.2.0-rc1+dev30 nasa/sample_lib#54, Replace <> with " for local includes nasa/sample_lib#53, Adds CONTRIBUTING.md that links to the main cFS contributing guide. nasa/cFS-GroundSystem#171 v2.2.0-rc1+dev41 nasa/cFS-GroundSystem#166, Updated TBL and SB tlm for an operational TLM display nasa/cFS-GroundSystem#170, Add Contributing Guide nasa/cFS-GroundSystem#137, Create package for cfs-groundsystem
nasa/cFE#1243 v6.8.0-rc1+dev436 nasa/cFE#1225, Add coverage test fix nasa/cFE#1218, bit order macros nasa/cFE#1193, Removes incorrect statements from Application Developers Guide nasa/cFE#1235, Fixes truncation handling on vsnprintf error by adding a cast to avoid implicit conversion nasa/cFE#1220, Clarify the documentation on SB MsgId regarding requirements for command and telemetry messages nasa/cFE#1230, Cast isspace input to unsigned char to avoid undefined behavior nasa/cFE#1231, Updated message module, msgid v1 to use mask instead of cast to alter value nasa/cFE#1232, Coercion alters value caused by incorrect type - static analysis warning nasa/cFE#1228, Replaces `<>` with `"` in local `#include`s nasa/cFE#1227, Adds `CONTRIBUING.md` that links to the main cFS contributing guide. nasa/PSP#273 v1.5.0-rc1+dev90 nasa/PSP#264, modular psp implementation nasa/PSP#272, Use quotes for local includes nasa/PSP#271, Add Contributing Guide nasa/osal#917 v5.1.0-rc1+dev347 nasa/osal#890, Move copyblock size to a #define and add comments nasa/osal#891, Removed rogue while loop nasa/osal#892, Scripted replacement for #include <os and #include <OSC_ matches of < and > with " nasa/osal#893, Consolidates the duplicated switch in OS_SocketOpen_Impl nasa/osal#894, Add `const` to input pointers nasa/osal#895, Removed network prototypes defined in osapi_sockets.h that are also in osapi_network.h nasa/osal#896, Removes NULL redefine from common_types.h nasa/osal#912, Add Contributing Guide nasa/osal#914, Utilize UTASSERT_CASETYPE_NA to report OS_ERR_NOT_IMPLEMENTED nasa/osal#898, correct interval calculation in DoSelect nasa/osal#909, comments describing select after connect nasa/osal#902, add check for EAGAIN in addition to EINTR nasa/osal#908, compile time assert for sockaddr size nasa/osal#910, check/report fcntl status nasa/osal#897, Add assert for FD_SET_SIZE in relation to OSAL_set nasa/osal#903, better error translation for ESPIPE errno nasa/osal#840, add shell functional test nasa/osal#901, rename OS_U32ValueWrapper_t nasa/osal#900, break up logic in return statement nasa/osal#906, return moduleInfoGet error nasa/osal#907, report timer_gettime error nasa/osal#899, remove unreachable test nasa/osal#905, make module comment same as other services nasa/osal#920 to fix test error check index inside fdset conversions nasa/osal#922, make non-selectable FD an error nasa/sample_app#137 v1.2.0-rc1+dev54 nasa/sample_app#134, Convert from <> to " for local includes nasa/sample_app#136, Added a contributing guide that links to the main cFS contributing guide. nasa/sample_app#132, Add context to the values for MsgIDs nasa/sample_lib#55 v1.2.0-rc1+dev30 nasa/sample_lib#54, Replace <> with " for local includes nasa/sample_lib#53, Adds CONTRIBUTING.md that links to the main cFS contributing guide. nasa/cFS-GroundSystem#171 v2.2.0-rc1+dev41 nasa/cFS-GroundSystem#166, Updated TBL and SB tlm for an operational TLM display nasa/cFS-GroundSystem#170, Add Contributing Guide nasa/cFS-GroundSystem#137, Create package for cfs-groundsystem
Describe the contribution
Fixes #1033
removes false child tasks statement.
Testing performed
Build and run unit test
Expected behavior changes
No impact to behavior
System(s) tested on
Ubuntu 20.04
Additional context
References to cFE Deployment Guide were fixed by #1166
Contributor Info - All information REQUIRED for consideration of pull request
Alex Campbell GSFC