Fix #642, 645, 701, 702, 703 - OSAL global table management #704
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Convert remaining operations using for loops to use iterators. This ensures locking is done consistently and correctly.
Implement an "unlock key" - based on task ID - which can be part of the local token, rather than relying on the task ID being the same between the lock and unlock ops. Notably, the task ID can change, in particular if the task is exiting. Also Fixes nasa#701, other general cleanup Implement all global lock/unlock error checking in shared layer, not in impl layer, for consistency. Remove redundant checks. Make all functions return void (should never fail) and in the unlikely event that something does fail then report the error, but no other recourse possible.
Change the EXCLUSIVE lock type such that it sets the ID in the global table to RESERVED and unlocks the global before returning to the caller. This allows the potentially long-running operation to complete and not block other operations from happening in other tasks. Use the EXCLUSIVE lock for all create/delete ops as well as for bind and connect socket ops. Also implement a new "RESERVED" lock to handle a special case in the vxworks timebase implementation where the impl layer needs to acquire a token for an object as it is being created. This case is special because it needs to happen during OS_TimeBaseCreate, and cannot be completed after the fact like normal tasks, because it is a factor in determining the success/fail status of the overall operation.
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.
|
Note this has a lot of the same content that was already reviewed in draft form on my previous PR #676, during CCB 2020-12-09, but broken out into several commits and also verified on VxWorks. Notably I had to add the "RESERVED" lock type to allow time bases to be created correctly - as these do in fact need to obtain a token on a record while it is in this intermediate state. |
Update unit tests for idmap functions, add test cases where coverage was incomplete. All OS_ObjectId* function coverage is back at 100%.
skliper
approved these changes
Dec 28, 2020
jphickey
added
the
CCB:Ready
astrogeco
added
CCB-20210106
and removed
CCB:Ready
|
CCB 2021-01-06 APPROVED |
astrogeco
added a commit
to nasa/cFS
that referenced
this pull request
Jan 12, 2021
jphickey
pushed a commit
to jphickey/osal
that referenced
this pull request
Aug 10, 2022
jphickey
pushed a commit
to jphickey/osal
that referenced
this pull request
Aug 10, 2022
Fix nasa#704, Added stub for CFE_SB_DeletePipe in ut_sb_stubs.c
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe the contribution
Address multiple issues with the OSAL global table management - general cleanup and bug fixes.
Fixes #702 - use iterators whenever possible
Fixes #645 - use an unlock key rather than task ID so OS_TaskExit() doesn't trigger a warning
Fixes #701 - general cleanup of lock/unlock impl and remove redundant logic
Fixes #703 - unlock global tables during create/delete
Fixes #642 - keep threads "attached" in POSIX, so they can be joined when deleted.
Testing performed
Build and run all tests for all platforms
Sanity check CFE and also confirmed RELOAD/RESTART commands are working correctly.
Expected behavior changes
No longer triggers warning with OS_TaskExit() on VxWorks (see #645)
OS_TaskDelete() on POSIX does not return until the task has actually exited (see #642)
Most other changes are internal only and do not change behavior.
System(s) tested on
Ubuntu 20.04 (native)
RTEMS 4.11.3 + pc686
VxWorks 6.9 + mcp750
Additional context
Noted other issues when running some unit tests on VxWorks target that are not related to this change / preexisting issues. Will file separate bugs about those.
Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.