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

CFE SB Pipes not safe across multiple tasks #1079

Open
jphickey opened this issue Jan 8, 2021 · 2 comments
Open

CFE SB Pipes not safe across multiple tasks #1079

jphickey opened this issue Jan 8, 2021 · 2 comments

Comments

@jphickey
Copy link
Contributor

jphickey commented Jan 8, 2021

Is your feature request related to a problem? Please describe.
Some "worker" software design patterns involve multiple threads reading from a common/shared work queue.

However due to the way SB buffers are managed, this is not currently possible with CFE child tasks and SB pipes. The "current" (i.e. most recent) buffer is stored in the Pipe descriptor structure, and upon the next entry to CFE_SB_ReceiveBuffer() this function assumes that the last buffer stored in the Pipe descriptor can be freed. But when multiple tasks are reading a single SB pipe, this model breaks, because only one buffer can be remembered. The current buffer is likely still in use by the other task when the next worker thread calls

This necessitates some new APIs to actually make this work. Each worker task will need to individually indicate to SB when it is actually done with the buffer, it can't rely on entry to to CFE_SB_ReceiveBuffer() to indicate this.

Describe the solution you'd like
Short term fix: Just document that only one task may operate on a pipe ID at a given time. App developers must externally sync their worker tasks to ensure this.

Recommendation would be to have one designated task (i.e. the main task) act as the delegater - it reads the SB pipe, identifies the request, and copies the request data to an available worker thread. After this it can get a new request from the SB pipe while the worker goes on.

Longer term fix: Expose the SB buffer refcount increment/decrement routines separately in the public API, and decouple the previous buffer refcount decrement from CFE_SB_ReceiveBuffer(). So each worker task can safely get a buffer from SB without inadvertently freeing any previous buffer that may be still in use by other worker tasks.

However, this is an API change that would affect all apps, as they now must make a new/additional call into CFE_SB when they are finished with a buffer as CFE_SB_ReceiveBuffer() cannot not do that automatically.

Describe alternatives you've considered
Could feasibly have a task-based buffer record so that each CFE task will have its own "slot" so to speak, and thereby CFE_SB_ReceiveBuffer could free the previous buffer from that task rather than having the single buffer associated with the pipe ID.

But this has weakness too- SB has to have a slot for every possible task whether it uses SB pipes or not. It also means the only way to free your previous buffer is to call CFE_SB_ReceiveBuffer() again and get a new buffer. So if the work is a long-running job it will "own" the buffer the entire time and prevent its re-use (and long-running jobs would likely be the reason for using a worker model in the first place)

Additional context
Found when reviewing race conditions in SB as part of #1073. Supporting multiple threads reading the same pipe might be a nice to have but will have inherent race conditions with the current API design. So to keep the API design as is for this cycle we will have to restrict this.

Requester Info
Joseph Hickey, Vantage Systems, Inc.

@jphickey
Copy link
Contributor Author

jphickey commented Jan 8, 2021

Related issue #216 - this is pretty much the same as the "longer term fix" above, looks like general use case for SB buffer refcount control was identified a long time ago.

@skliper
Copy link
Contributor

skliper commented Jan 8, 2021

Started searching for where I saw the "worker" use case.. but I did find the restriction documented here:

cFE/docs/src/cfe_sb.dox

Lines 100 to 102 in 1ede295

PipeId) is given back to the caller after the API is executed. Each pipe can be
read by only one task, but a task may read more than one pipe. Only the pipe
owner is allowed to subscribe to messages on the pipe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants