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

Fix #642, synchronous task delete on POSIX #676

Closed

Conversation

jphickey
Copy link
Contributor

@jphickey jphickey commented Dec 9, 2020

Describe the contribution
Make OS_TaskDelete actually wait for the task to exit, not simply a cancellation request.

Fix #642

Testing performed
Build and run CFE and all unit tests
Test the CFE "reload app" command as documented in nasa/cFE#952 which depends on having OS_TaskDelete actually exit the task

Expected behavior changes
No impact to external API/observed behavior.

Internally, the global tables are now unlocked when using an EXCLUSIVE lock type, which is used for create and delete ops. Instead of holding the global lock, the ID is set to "RESERVED" (FFFFFFFF) which effectively prevents any other task from obtaining a reference to that object. This way OSAL can more safely call C library functions which block, including (but not limited to) pthread_join, while allowing ops on other records to proceed.

System(s) tested on
Ubuntu 20.04
RTEMS 4.11

Additional context
Required as part of the fix for CFE requirement documented in nasa/cFE#952.

Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.

@jphickey jphickey changed the base branch from main to integration-candidate December 9, 2020 13:58
@jphickey jphickey marked this pull request as draft December 9, 2020 13:59
@jphickey jphickey added the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Dec 9, 2020
@jphickey
Copy link
Contributor Author

jphickey commented Dec 9, 2020

Only marked as draft because it requires a baseline which is not yet in main - will rebase once next baseline is settled.

@astrogeco
Copy link
Contributor

CCB 2020-12-09 APPROVED

@astrogeco astrogeco added CCB-20201209 and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Dec 10, 2020
In the POSIX implementation, OS_TaskDelete was implemented in a
deferred manner - the API call was a request, and the actual
deletion occured sometime thereafter.  This is a problem if the
task is running code within a dynamically loaded module, and the
intent is to delete the task so the module can be unloaded.  In
this case the app needs to be certain that the task has actually
been deleted before unloading can be done safely.

To do this requires use of pthread_join() on POSIX which confirms
that the task has exited.  However, this is a (potentially) blocking
call, so to do this requires rework of the EXCLUSIVE lock mode
such that the OSAL lock is _not_ held during the join operation.
@jphickey jphickey marked this pull request as ready for review December 11, 2020 00:42
@jphickey
Copy link
Contributor Author

Rebased to main, and also revalidated that all platforms build and test passes.

NOTE: in order to pass tests this needs #671 also merged (dependency). Without this change then OS_TaskDelete() fails.

* a) we want to increment refcount but an exclusive is pending
* b) we want exclusive but refcount is nonzero
* c) we want exclusive but another exclusive is pending
* a) we want to some type of lock but the ID is currently RESERVED
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

@skliper skliper linked an issue Dec 16, 2020 that may be closed by this pull request
@jphickey
Copy link
Contributor Author

@astrogeco -- testing a preemptive merge on VxWorks and noted an issue with creating timers. May want to hold off on merging this, I will have to add a patch of some type.

@jphickey jphickey marked this pull request as draft December 16, 2020 16:26
@jphickey
Copy link
Contributor Author

I have fixed the issues on VxWorks related to this and also broke it up into smaller commits.
Closing this PR, see PR #704 instead which has all this stuff plus some more fixes.

@jphickey jphickey closed this Dec 22, 2020
@jphickey jphickey deleted the fix-642-sync-taskdelete branch April 29, 2021 13:22
jphickey added a commit to jphickey/osal that referenced this pull request Aug 10, 2022
Should be CFE_MISSION_SPACECRAFT_ID
jphickey pushed a commit to jphickey/osal that referenced this pull request Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OS_TaskDelete is not synchronous on POSIX
3 participants