Skip to content

fix(sync): clear connect state when the remote aborts a dial as already-syncing - #111

Open
vladimirlogachev wants to merge 1 commit into
n0-computer:mainfrom
vladimirlogachev:main
Open

vladimirlogachev wants to merge 1 commit into
n0-computer:mainfrom
vladimirlogachev:main

Conversation

@vladimirlogachev

Copy link
Copy Markdown

Description

When our dial is rejected by the remote with AbortReason::AlreadySyncing, on_sync_via_connect_finished skips on_sync_finished, so NamespaceStates::finish never runs for that exchange. If no concurrent incoming exchange took the slot over (mutual-dial tie-break), the (namespace, peer) pair stays Running forever and every later sync trigger for it — NewNeighbor, SyncReport, Resync — is silently dropped in start_connect. The rejection is easy to hit when one side redials right after an exchange while the other side has not yet processed its finish event; downstream we observed pairs permanently stopping to sync, caught by a scenario stress test failing about once in 80 runs.

Changes:

  • engine/state.rs: add NamespaceStates::abort_connect, clearing the running outgoing exchange back to Idle — guarded so it only clears a Running { origin: Connect(reason) } slot with the same SyncReason; an accept exchange that took the slot over in the tie-break is left untouched (it finishes through the accept path), as is a newer dial with a different reason.
  • engine/live.rs: call it from the RemoteAbort(AlreadySyncing) arm of on_sync_via_connect_finished.
  • Three unit tests on the state machine: the rejected dial returns the pair to idle; a stale abort does not clear a newer dial; an accept takeover is spared.

Breaking Changes

None — additive internal change to the live engine's sync-state tracking; no public API or wire format touched.

Notes & open questions

The guard distinguishes exchanges by SyncReason, not by session identity — a late abort of an old dial can clear a newer dial started for the same reason. The consequence is an extra idle transition (the next trigger restarts the exchange), not a wedge. If you want it stricter, SyncState::Running would need a session id; happy to extend if preferred.

Change checklist

  • Self-review.
  • Documentation updates following the style guide, if relevant.
  • Tests if relevant.
  • All breaking changes documented.

@n0bot n0bot Bot added this to iroh Jul 14, 2026
@github-project-automation github-project-automation Bot moved this to 🚑 Needs Triage in iroh Jul 14, 2026

This branch has not been deployed

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

Labels

None yet

Projects

Status: 🚑 Needs Triage

Development

Successfully merging this pull request may close these issues.

1 participant