Skip to content

fix(iroh): bound retry and lookup lifecycles - #1

Open
azooz2003-bit wants to merge 5 commits into
mainfrom
fix/cmux-iroh-1.0-bounds
Open

fix(iroh): bound retry and lookup lifecycles#1
azooz2003-bit wants to merge 5 commits into
mainfrom
fix/cmux-iroh-1.0-bounds

Conversation

@azooz2003-bit

@azooz2003-bit azooz2003-bit commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Description

Hardens two per-peer actor lifecycles in the cmux Iroh v1.0 fork.

  • Deduplicate pending path-open retries across connections.
  • Cap retries at 64 distinct paths with oldest-first eviction.
  • Give each address resolution an explicit cancellation lifetime.
  • Drop a hanging lookup when its final waiting dial is cancelled and no path or connection remains.
  • Preserve a shared lookup while another dial is still waiting.

Both fixes use red-test then green-fix commit pairs.

Breaking Changes

None.

Residual risk

Persistent path-ID exhaustion can still produce bounded retry CPU. The cancellation path is scoped to address resolution and does not alter established connection lifetime.

Verification

  • 117 Iroh tests passed, 1 known flaky test ignored.
  • 1 integration test and 26 doctests passed, 2 doctests ignored.
  • All-feature clippy passed with warnings denied.
  • Formatting and diff checks passed.

View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.

Summary by CodeRabbit

  • Bug Fixes

    • Cancelled connection attempts now promptly stop in-progress address lookups.
    • Prevented cancelled resolution requests from affecting other active requests.
    • Added safeguards to keep pending connection retries bounded and deduplicated.
    • Improved cleanup of abandoned address-resolution operations to prevent resource buildup.
  • Tests

    • Expanded coverage for cancellation, cleanup, concurrent resolution requests, and bounded retry behavior.

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 53b94387-9a81-4b03-b4d7-607eac41fb69

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

A CancellationToken is threaded through the address-resolution path: Socket creates and passes it via ActorMessage::ResolveRemote into RemoteMap and RemoteStateActor, which now tracks cancellations, prunes cancelled queued resolves in RemotePathState, and bounds/deduplicates pending open-path retries. Tests validate drop-triggered cancellation and queue behavior.

Changes

Cancellable resolve_remote propagation

Layer / File(s) Summary
Socket entry point creates and forwards cancellation token
iroh/src/socket.rs
Socket::resolve_remote creates a CancellationToken with a drop guard, sends it in ActorMessage::ResolveRemote, and the actor handler forwards it to remote_map.resolve_remote.
RemoteMap forwards cancellation to actor
iroh/src/socket/remote_map.rs
RemoteMap::resolve_remote gains a CancellationToken parameter forwarded in RemoteStateMessage::ResolveRemote; tests updated to pass tokens.
RemoteStateActor tracks and reacts to cancelled resolves
iroh/src/socket/remote_map/remote_state.rs
Adds address_lookup_cancellations and a bounded/deduped PendingOpenPaths queue, registers cancellation futures in handle_msg_resolve_remote, prunes cancelled requests via handle_resolve_cancellation, switches retry scheduling to enqueue, and adds bound-check unit tests.
RemotePathState queues PendingResolveRequest with cancellation
iroh/src/socket/remote_map/remote_state/path_state.rs
Pending resolve queue now stores PendingResolveRequest (reply + CancellationToken); resolve_remote returns queued status; prune_cancelled_resolve_requests removes stale entries; tests updated and extended.
End-to-end drop test for hanging address lookup
iroh/src/address_lookup.rs
Test module adds sync imports and reworks a test to probe start/drop of a hanging resolve future, verifying cancellation propagates end-to-end.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Socket
  participant RemoteMap
  participant RemoteStateActor
  participant RemotePathState

  Socket->>Socket: create CancellationToken + drop_guard
  Socket->>RemoteMap: resolve_remote(addr, tx, cancellation)
  RemoteMap->>RemoteStateActor: ResolveRemote(addrs, tx, cancellation)
  RemoteStateActor->>RemotePathState: resolve_remote(tx, cancellation)
  RemotePathState-->>RemoteStateActor: queued = true/false
  RemoteStateActor->>RemoteStateActor: register cancellation.cancelled_owned()

  alt caller drops request
    Socket->>Socket: drop_guard triggers cancellation
    RemoteStateActor->>RemoteStateActor: cancellation future resolves
    RemoteStateActor->>RemotePathState: prune_cancelled_resolve_requests()
    RemoteStateActor->>RemoteStateActor: stop address-lookup stream if empty
  else resolution completes
    RemotePathState->>RemoteStateActor: emit_pending_resolve_requests()
    RemoteStateActor->>RemoteMap: reply via oneshot
  end
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: bounding retry and lookup lifecycles.
Description check ✅ Passed The description covers the main changes, breaking changes, risks, and verification, but it omits the notes/open questions and checklist sections.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/cmux-iroh-1.0-bounds

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@azooz2003-bit azooz2003-bit changed the title fix(iroh): bound path-open retry queue fix(iroh): bound retry and lookup lifecycles Jul 10, 2026
@azooz2003-bit
azooz2003-bit marked this pull request as ready for review July 10, 2026 07:38
@azooz2003-bit

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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.

1 participant