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

Introduce transaction object for CFE TBL #2538

Closed
jphickey opened this issue Mar 28, 2024 · 0 comments · Fixed by #2539
Closed

Introduce transaction object for CFE TBL #2538

jphickey opened this issue Mar 28, 2024 · 0 comments · Fixed by #2539
Assignees

Comments

@jphickey
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Currently the table services implementation has a number of likely race conditions. Most API calls do not lock the registry - generally only handle registration does so, and only for the specific part of the operation that finds the empty slot.

This is not an easy fix, as most functions are complex and scattered with inline "return" calls.

Describe the solution you'd like
Introduce the concept of a transaction. A similar concept is used in OSAL and other CFS apps.

A transaction will track the overall state of the change. Instead of independently tracking the Handle, DescPtr and RegRegPtr in the code, the transaction object wraps it all into one struct. It can also track what was changed, such that it can be reliably un-done if something later in the process did not work. For example, allocating a new table requires both a registry entry and an access descriptor. If the registry entry is successfully allocated first, and then the access descriptor fails to allocate, then the registry entry must be released to put the global tables back into their initial state.

Importantly, if a global object is locked during a transaction, it must be unlocked at the end of the transaction. The transaction object can track this state as well.

Additional context
This would be just one step toward fixing race conditions. In order to properly fix these, logging and event generation will likely need to be moved to the end of the transaction (after unlock). Importantly, we can't just extend locks across the entire operation, if the operation involves calls into other subsystems to write to syslog or send an event. But this will likely help reveal where the problem areas are and make it easier to fix.

Requester Info
Joseph Hickey, Vantage Systems, Inc.

@jphickey jphickey self-assigned this Mar 28, 2024
jphickey added a commit to jphickey/cFE that referenced this issue Mar 28, 2024
Initial implementation of transaction concept for table API calls.
The basic transaction object wraps all of the relevant pointers
and state information for the request being processed.
jphickey added a commit to jphickey/cFE that referenced this issue Mar 28, 2024
Initial implementation of transaction concept for table API calls.
The basic transaction object wraps all of the relevant pointers
and state information for the request being processed.
jphickey added a commit to jphickey/cFE that referenced this issue Mar 28, 2024
Initial implementation of transaction concept for table API calls.
The basic transaction object wraps all of the relevant pointers
and state information for the request being processed.
jphickey added a commit to jphickey/cFE that referenced this issue Mar 28, 2024
Initial implementation of transaction concept for table API calls.
The basic transaction object wraps all of the relevant pointers
and state information for the request being processed.
jphickey added a commit to jphickey/cFE that referenced this issue Mar 28, 2024
Initial implementation of transaction concept for table API calls.
The basic transaction object wraps all of the relevant pointers
and state information for the request being processed.
dzbaker added a commit that referenced this issue Mar 28, 2024
Fix #2538, table transaction initial implementation
@dzbaker dzbaker closed this as completed in 0604676 Apr 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant