Skip to content

Remove Timeout from Svc::FileDownlink - #4555

Merged
thomas-bc merged 3 commits into
nasa:develfrom
LeStarch:issue-4550
Dec 16, 2025
Merged

Remove Timeout from Svc::FileDownlink#4555
thomas-bc merged 3 commits into
nasa:develfrom
LeStarch:issue-4550

Conversation

@LeStarch

Copy link
Copy Markdown
Collaborator
Related Issue(s) #4550
Has Unit Tests (y/n) y
Documentation Included (y/n) y
Generative AI was used in this contribution (y/n) y

Change Description

Svc.FileDownlink has a buffer ownership bug that causes a buffer to be owned by multiple owners. This can cause unknown file downlink corruption.

The fix has to be for timeout to wait for buffer return, which undermines the need for a timeout at all. Thus we remove the timeout.

@LeStarch
LeStarch requested a review from thomas-bc December 15, 2025 20:32
@thomas-bc
thomas-bc merged commit 4f3568d into nasa:devel Dec 16, 2025
59 of 62 checks passed
@thomas-bc
thomas-bc deleted the issue-4550 branch December 16, 2025 00:50
nathancheek added a commit to nathancheek/fprime that referenced this pull request Jun 12, 2026
* Backport nasa#4555 to v3.6.x
* Ignore returned buffers when in COOLDOWN mode, matching IDLE mode behavior
* Spelling fix

---------

Co-authored-by: M Starch <LeStarch@googlemail.com>
nathancheek added a commit to nathancheek/fprime that referenced this pull request Jun 12, 2026
* Backport nasa#4555 to v3.6.x
* Ignore returned buffers when in COOLDOWN mode, matching IDLE mode behavior
* Spelling fix

---------

Co-authored-by: M Starch <LeStarch@googlemail.com>
nathancheek added a commit to nathancheek/fprime that referenced this pull request Jun 12, 2026
* Backport nasa#4555 to v3.6.x
* Ignore returned buffers when in COOLDOWN mode, matching IDLE mode behavior
* Spelling fix
* Update Ref topology to match changes to FileDownlink::config()

---------

Co-authored-by: M Starch <LeStarch@googlemail.com>
nathancheek added a commit to nathancheek/fprime that referenced this pull request Jun 18, 2026
* Backport nasa#4555 to v3.6.x
* Ignore returned buffers when in COOLDOWN mode, matching IDLE mode behavior
* Spelling fix
* Update Ref topology to match changes to FileDownlink::config()

---------

Co-authored-by: M Starch <LeStarch@googlemail.com>
nathancheek added a commit to nathancheek/fprime that referenced this pull request Jun 18, 2026
* Backport nasa#4555 to v3.6.x
* Ignore returned buffers when in COOLDOWN mode, matching IDLE mode behavior
* Spelling fix
* Update Ref topology to match changes to FileDownlink::config()

---------

Co-authored-by: M Starch <LeStarch@googlemail.com>
LeStarch added a commit that referenced this pull request Jun 27, 2026
* Backport #4555 to v3.6.x
* Ignore returned buffers when in COOLDOWN mode, matching IDLE mode behavior
* Spelling fix
* Update Ref topology to match changes to FileDownlink::config()

---------

Co-authored-by: M Starch <LeStarch@googlemail.com>
rvaccone added a commit to rvaccone/fprime that referenced this pull request Aug 15, 2026
…Transfer declaration

All three are leftovers from the timeout removal in nasa#4555: m_timeout and
m_bufferSize are never written or read, and exitFileTransfer has no
definition.
rvaccone added a commit to rvaccone/fprime that referenced this pull request Aug 15, 2026
A downlink stuck in WAIT (or CANCEL) because the downstream component
never returns the outstanding buffer produces no fault indication:
pingIn still responds, so Svc.Health cannot see the failure. Add an
optional stall timeout to configure(); when a downlink has waited that
long for a buffer return, emit a DownlinkStalled warning once per wait.
The default of 0 disables the warning, and existing configure() callers
are unaffected.

The warning is observational only. It triggers no automatic action, so
it does not reintroduce the automatic timeout removed in nasa#4555; it tells
operators that Reset may be needed. It is deliberately not count-throttled:
the emission is already rate-limited to one per wait by construction, and
a count throttle could be exhausted by a slow link and then stay silent
through the very wedge the event exists to report.

FileHandlingConfig gains a stallTimeout constant, defaulted to 0 and
passed through by the FileHandling subtopology, so deployments can opt in
by overriding config rather than by calling configure() a second time
(which would hit ALREADY_CREATED on the file queue and assert at startup).
rvaccone added a commit to rvaccone/fprime that referenced this pull request Aug 23, 2026
…Transfer declaration

All three are leftovers from the timeout removal in nasa#4555: m_timeout and
m_bufferSize are never written or read, and exitFileTransfer has no
definition.
rvaccone added a commit to rvaccone/fprime that referenced this pull request Aug 23, 2026
A downlink stuck in WAIT (or CANCEL) because the downstream component
never returns the outstanding buffer produces no fault indication:
pingIn still responds, so Svc.Health cannot see the failure. Add an
optional stall timeout to configure(); when a downlink has waited that
long for a buffer return, emit a DownlinkStalled warning once per wait.
The default of 0 disables the warning, and existing configure() callers
are unaffected.

The warning is observational only. It triggers no automatic action, so
it does not reintroduce the automatic timeout removed in nasa#4555; it tells
operators that Reset may be needed. It is deliberately not count-throttled:
the emission is already rate-limited to one per wait by construction, and
a count throttle could be exhausted by a slow link and then stay silent
through the very wedge the event exists to report.

FileHandlingConfig gains a stallTimeout constant, defaulted to 0 and
passed through by the FileHandling subtopology, so deployments can opt in
by overriding config rather than by calling configure() a second time
(which would hit ALREADY_CREATED on the file queue and assert at startup).
rvaccone added a commit to rvaccone/fprime that referenced this pull request Aug 30, 2026
…Transfer declaration

All three are leftovers from the timeout removal in nasa#4555: m_timeout and
m_bufferSize are never written or read, and exitFileTransfer has no
definition.
rvaccone added a commit to rvaccone/fprime that referenced this pull request Aug 30, 2026
A downlink stuck in WAIT (or CANCEL) because the downstream component
never returns the outstanding buffer produces no fault indication:
pingIn still responds, so Svc.Health cannot see the failure. Add an
optional stall timeout to configure(); when a downlink has waited that
long for a buffer return, emit a DownlinkStalled warning once per wait.
The default of 0 disables the warning, and existing configure() callers
are unaffected.

The warning is observational only. It triggers no automatic action, so
it does not reintroduce the automatic timeout removed in nasa#4555; it tells
operators that Reset may be needed. It is deliberately not count-throttled:
the emission is already rate-limited to one per wait by construction, and
a count throttle could be exhausted by a slow link and then stay silent
through the very wedge the event exists to report.

FileHandlingConfig gains a stallTimeout constant, defaulted to 0 and
passed through by the FileHandling subtopology, so deployments can opt in
by overriding config rather than by calling configure() a second time
(which would hit ALREADY_CREATED on the file queue and assert at startup).
rvaccone added a commit to rvaccone/fprime that referenced this pull request Sep 3, 2026
…Transfer declaration

All three are leftovers from the timeout removal in nasa#4555: m_timeout and
m_bufferSize are never written or read, and exitFileTransfer has no
definition.
rvaccone added a commit to rvaccone/fprime that referenced this pull request Sep 3, 2026
A downlink stuck in WAIT (or CANCEL) because the downstream component
never returns the outstanding buffer produces no fault indication:
pingIn still responds, so Svc.Health cannot see the failure. Add an
optional stall timeout to configure(); when a downlink has waited that
long for a buffer return, emit a DownlinkStalled warning once per wait.
The default of 0 disables the warning, and existing configure() callers
are unaffected.

The warning is observational only. It triggers no automatic action, so
it does not reintroduce the automatic timeout removed in nasa#4555; it tells
operators that Reset may be needed. It is deliberately not count-throttled:
the emission is already rate-limited to one per wait by construction, and
a count throttle could be exhausted by a slow link and then stay silent
through the very wedge the event exists to report.

FileHandlingConfig gains a stallTimeout constant, defaulted to 0 and
passed through by the FileHandling subtopology, so deployments can opt in
by overriding config rather than by calling configure() a second time
(which would hit ALREADY_CREATED on the file queue and assert at startup).
rvaccone added a commit to rvaccone/fprime that referenced this pull request Sep 7, 2026
…Transfer declaration

All three are leftovers from the timeout removal in nasa#4555: m_timeout and
m_bufferSize are never written or read, and exitFileTransfer has no
definition.
rvaccone added a commit to rvaccone/fprime that referenced this pull request Sep 7, 2026
A downlink stuck in WAIT (or CANCEL) because the downstream component
never returns the outstanding buffer produces no fault indication:
pingIn still responds, so Svc.Health cannot see the failure. Add an
optional stall timeout to configure(); when a downlink has waited that
long for a buffer return, emit a DownlinkStalled warning once per wait.
The default of 0 disables the warning, and existing configure() callers
are unaffected.

The warning is observational only. It triggers no automatic action, so
it does not reintroduce the automatic timeout removed in nasa#4555; it tells
operators that Reset may be needed. It is deliberately not count-throttled:
the emission is already rate-limited to one per wait by construction, and
a count throttle could be exhausted by a slow link and then stay silent
through the very wedge the event exists to report.

FileHandlingConfig gains a stallTimeout constant, defaulted to 0 and
passed through by the FileHandling subtopology, so deployments can opt in
by overriding config rather than by calling configure() a second time
(which would hit ALREADY_CREATED on the file queue and assert at startup).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants