Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failure during CFE_ES_ExitApp if app calls CFE_TBL_Unregister #741

Closed
pcooksey opened this issue Jun 12, 2020 · 5 comments · Fixed by #1602 or #1619
Closed

Failure during CFE_ES_ExitApp if app calls CFE_TBL_Unregister #741

pcooksey opened this issue Jun 12, 2020 · 5 comments · Fixed by #1602 or #1619
Assignees
Labels
bug cFE-TBL Table services docs This change only affects documentation.
Milestone

Comments

@pcooksey
Copy link

Describe the bug
Should an app call CFE_TBL_Unregister?
On CFE_ES_ExitApp I get:

1980-012-14:03:21.25234 CFE_TBL:RemoveAccessLink-PutPoolBuf[0] Fail Stat=0xC4000007, Hndl=0x56709730, Buf=0x00000000
1980-012-14:03:21.25236 CFE_TBL:RemoveAccessLink-PutPoolBuf[0] Fail Stat=0xC4000007, Hndl=0x56709730, Buf=0x00000000

The documentation no longer says anything about it so I'm starting to think it shouldn't be used. The sample_app in the cFS repo also doesn't call CFE_TBL_Unregister either.

The code seems to say we should though:

** \par Description
** When an application is being removed from the system, it should
** unregister those tables that it created. The application should
** call this function as a part of its cleanup process. The table
** will be removed from memory once all table addresses referencing
** it have been released.

To Reproduce

  1. Register table
  2. Unregister table
  3. Call CFE_ES_ExitApp

Expected behavior
No errors on exit

Code snips
N/A

System observed on:

  • Hardware: Laptop
  • OS: Linux
  • Versions: Versions:cFE 6.7.7.0, OSAL 5.0.6.0, PSP 1.4.4.0

Additional context
N/A

Reporter Info
Philip Cooksey, NASA Ames

@pcooksey
Copy link
Author

It seems by default the tables are being cleaned up automatically as shown below which is part of the CFE_ES_CleanUpApp function:

#ifndef EXCLUDE_CFE_TBL
CFE_TBL_CleanUpApp(AppId);
#endif

This clean up call eventually calls:

CFE_TBL_RemoveAccessLink(i);

Which is the same function in CFE_TBL_Unregister:

CFE_TBL_RemoveAccessLink(TblHandle);

Seems like a bug since an already unregistered table shouldn't be cleaned up again by CFE_ES_CleanUpApp -> CFE_TBL_CleanUpApp.

@skliper skliper added the bug label Jun 17, 2020
@skliper skliper changed the title Should an app call CFE_TBL_Unregister? Failure during CFE_ES_ExitApp if app calls CFE_TBL_Unregister Jun 17, 2020
@skliper
Copy link
Contributor

skliper commented Jun 17, 2020

Seems like a bug since an already unregistered table shouldn't be cleaned up again by CFE_ES_CleanUpApp -> CFE_TBL_CleanUpApp.

Agreed. Updated title and marked as bug.

@skliper skliper added this to the 7.0.0 milestone Jan 11, 2021
@jphickey
Copy link
Contributor

Tested this today - It may have been fixed by other changes, possibly the transition to ES background task and the better locking ES that was implemented with that.

To test - what I did was modify SAMPLE_APP (which registers a table) to only run for 5 seconds, then exit. As part of exiting, I added a call to CFE_TBL_Unregister() before it calls CFE_ES_ExitApp() ... I confirmed that the first call to CFE_TBL_Unregister is indeed doing the un-registration, followed by ES invoking CFE_TBL_CleanUpApp() via the background task - which did nothing in my test.

The only (minor) issue I observed is that there is an extra "unknown state" event after the app exits itself:
EVS Port1 66/1/CFE_ES 48: ES_ProcControlReq: Unknown State Application SAMPLE_APP ( 1 )

But otherwise everything seemed to work just fine.

@jphickey
Copy link
Contributor

@pcooksey can you check if this issue still exists in the current main branch?

@skliper
Copy link
Contributor

skliper commented Jan 25, 2021

If it's fixed we should still fix the documentation, table will be unregistered during cleanup so this is only needed if the App wants to unregister explicitly.

@skliper skliper added the docs This change only affects documentation. label Jan 25, 2021
@astrogeco astrogeco added the cFE-TBL Table services label Jan 27, 2021
@skliper skliper assigned skliper and unassigned jphickey Jun 3, 2021
skliper added a commit to skliper/cFE that referenced this issue Jun 3, 2021
skliper added a commit to skliper/cFE that referenced this issue Jun 3, 2021
astrogeco added a commit that referenced this issue Jun 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cFE-TBL Table services docs This change only affects documentation.
Projects
None yet
4 participants