Skip to content

Backport FileDownlink timeout removal to v3.6 #5287

Description

@nathancheek
F´ Version v3.6.4
Affected Component Svc.FileDownlink

Problem Description

A buffer ownership bug was fixed in v4.2.0, see #4555. It would be nice to backport this fix to v3.6.x.

This bug was found by implementing and placing a ThrottledBufferRepeater downstream of a FileDownlink component, and setting the throttle timing high enough that it triggered a FileDownlink timeout. This brought to light a bug in bufferReturn_handler(), where FileDownlink doesn't properly handle receiving buffers while in COOLDOWN mode.

The fix in #4555 should keep us from hitting the COOLDOWN bug in normal conditions, since FileDownlink won't enter COOLDOWN mode until the buffer has been returned. However, I would also like to update bufferReturn_handler() so it knows how to handle returned buffers while in COOLDOWN mode. Here's how bufferReturn_handler() currently handles returned buffers based on the component's mode:

  • IDLE: Does nothing
  • CANCEL: Sends appropriate next packet
  • WAIT: Sends appropriate next packet
  • DOWNLINK: asserts, but this is okay since the DOWNLINK mode is a temporary mode only used while inside of a bufferReturn_handler() call, so we should never be in DOWNLINK mode at the start of a bufferReturn_handler() call
  • COOLDOWN: asserts, not ideal! But would require the downstream component to return a packet at an unexpected time.

I think returned buffers should be handled the same way in both IDLE mode and COOLDOWN mode. Seeing as how bufferReturn_handler() already has a non-asserting offramp for buffers returned during IDLE mode, I propose we do the same for COOLDOWN mode. I believe this COOLDOWN fix would also be useful for F Prime v4... if there is any concurrence I can open an issue and PR to pull that change into dev.

I will open a PR shortly with the #4555 fix cherry-picked on top of v3.6.4, along with a fix so bufferReturn_handler() handles COOLDOWN mode.

How to Reproduce

  1. Place a component downstream of FileDownlink that takes longer than FileDownlink's m_timeout, but shorter than m_timeout + m_cooldown, to return.
  2. Once the timeout occurs, FileDownlink will assert in bufferReturn_handler() when it receives the returned buffer while in COOLDOWN mode.

Expected Behavior

FileDownlink should not assert when it receives a returned buffer while in COOLDOWN mode.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions