fix(ble): bound the flow-control credit wait so a wedged peer can't hang Write#161
Merged
Conversation
…ang Write BLE Writes split into chunks, each waiting until the firmware has advertised enough flow-control credit (FlowCtrl notifications). consumeCredit parked in creditCond.Wait() with NO deadline: if the firmware stops publishing credit — a wedged board or a silently-dropped link — the wait never returns and Write blocks forever. Only Close could unblock it; the caller's intent (and any turn-level cancellation) was ignored because Write has no ctx (io.Writer). consumeCredit now bounds the wait with bleCreditStallTimeout (15s). It still wakes immediately on a credit refresh or Close via creditCond, but a deadline caps the total wait: exceeding it returns a stall error instead of hanging. 15s is conservative — a healthy board refreshes credit at its sub-second BT scheduling tick, so this never false-positives on a busy-but-live board; it only fires when the peer is genuinely gone. The timeout is overridable via the creditStallTimeout field so tests exercise the stall path without a 15s wait. Found via the internal audit sweep (final backlog item). Tests: - TestConsumeCredit_StallTimesOut: no credit ever arrives -> returns a stall error within the (test-shortened) timeout. Proven to HANG (2s test deadline) on the old unbounded code. - TestConsumeCredit_CloseWakesWaiter: Close while parked -> os.ErrClosed promptly (not after the stall timeout). - TestConsumeCredit_CreditArrivesSucceeds: a fresh credit notification lets the waiter proceed and deduct. Verified: task ci green (lint 0 / vet / build / test:full 0 fail / govulncheck clean); task eval 17/17.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A BLE Write can hang forever on flow-control credit (category C, robustness). Writes split into chunks, each waiting until the firmware advertises enough credit via FlowCtrl notifications.
consumeCreditparked increditCond.Wait()with no deadline — so if the firmware stops publishing credit (a wedged board or a silently-dropped link), the wait never returns andWriteblocks forever. OnlyClosecould unblock it; the caller's intent (and any turn-level cancellation) was ignored, becauseWritehas no ctx (io.Writer).Fix
consumeCreditnow bounds the wait withbleCreditStallTimeout(15s). It still wakes immediately on a credit refresh orCloseviacreditCond, but a deadline caps the total wait — exceeding it returns a stall error instead of hanging. 15s is conservative: a healthy board refreshes credit at its sub-second BT scheduling tick, so it never false-positives on a busy-but-live board; it fires only when the peer is genuinely gone. Overridable via thecreditStallTimeoutfield so tests exercise the stall path without a 15s wait.Found via the internal audit sweep (final backlog item).
Verification
TestConsumeCredit_StallTimesOut: no credit ever arrives → stall error within the timeout. Proven to HANG (2s test deadline) on the old unbounded code.TestConsumeCredit_CloseWakesWaiter:Closewhile parked →os.ErrClosedpromptly.TestConsumeCredit_CreditArrivesSucceeds: a fresh credit notification lets the waiter proceed and deduct.task cigreen (lint 0 / vet / build /test:full0 fail / govulncheck clean);task eval17/17.Robustness fix — anchors v0.779.0; closes the audit-sweep backlog.