fix: give connect attempts an identity so stale failures can't clobber newer ones - #102
Merged
Merged
Conversation
…r newer ones Connect bookkeeping (watchdog, announce flag, result waiters) is keyed by address alone, and overlapping attempts are reachable: a silent watcher retry can still be dying — up to the 60s pair watchdog — when a peer command starts a fresh attempt for the same address. Its late failure then cancelled the new attempt's watchdog, consumed its announce flag, and failed its waiters (acking OP_FAILED mid-handoff, so the sender rolled back while the receiver's attempt continued). Failure paths now carry an attempt token and no-op once superseded. The pair delegate keys failures on the pair's own token (recorded when the pair is installed — the address's current token could belong to a newer attempt that hasn't installed its pair yet), token recording is newest-wins across threads, and a pairing success stays terminal-good: it opens and adopts the connection even when its attempt was superseded or its watchdog fired a beat earlier.
MegaManSec
force-pushed
the
fix/connect-attempt-identity
branch
from
August 7, 2026 06:59
d2f4556 to
5478640
Compare
MegaManSec
changed the title
|
🎉 This PR is included in version 2.25.4 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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.
From the adversarial review sweep (confirmed finding). Connect bookkeeping (pair watchdog, announce flag, result waiters, row state) is keyed by address alone, and overlapping attempts are reachable: a silent watcher/adoption retry can still be dying — up to the 60s watchdog — when a peer command starts a fresh attempt for the same address. The stale attempt's late failure then cancelled the new attempt's watchdog, consumed its announce flag, flipped its row to disconnected, and failed its waiters — acking OP_FAILED mid-handoff so the sender rolled back while the receiver's attempt kept running, leaving both Macs fighting over the set.
Every attempt now mints a monotonic token; failure paths carry theirs and no-op once superseded. Three refinements from the review of the first draft: the pair delegate fails with the pair's own token (recorded at install — the address's current token can belong to a newer attempt that hasn't installed its pair yet), token recording is newest-wins across threads, and a pairing success stays terminal-good — it opens and adopts the connection even if its attempt was superseded or its watchdog fired a beat earlier (the pre-token code recovered that race, and now it still does).
Stacked on #99 (
fix/bug-sweep) — merge that first, then retarget this to main.Test: normal single connect/release/take unaffected; take a peripheral from the peer while a watcher retry is mid-flight against a stuck device (the take must survive the retry's death: no spurious "Pairing Failed", no OP_FAILED rollback); a genuinely stuck device still times out with one announced "Pairing Timed Out". Typechecked here; please build before merging.