nasa / cFE

The Core Flight System (cFS) Core Flight Executive (cFE)
Apache License 2.0
409 stars 202 forks source link

Introduce transaction object for CFE TBL #2538

Closed jphickey closed 6 months ago

jphickey commented 6 months ago

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.