Skip to content

Conversation

@Frando
Copy link
Member

@Frando Frando commented Nov 10, 2025

Description

Alternative to #3631

Replaces the Watchables for path changes on the Connection with a boxed Watcher. The watcher is boxed because it would increase the Connection struct size significantly otherwise because the mapped-and-joined watcher with a SmallVec of PathInfo inside is ~600 bytes atm.

The benefit of storing a Watcher and not a Watchable is that the watcher streams now close once the EndpointStateActor drops the state for the connection, which it does after the connection is closed.

Also adds a test for path watching, including testing that the streams now close when the connection closes.

Breaking Changes

Notes & open questions

Change checklist

  • Self-review.
  • Documentation updates following the style guide, if relevant.
  • Tests if relevant.
  • All breaking changes documented.
    • List all breaking changes in the above "Breaking Changes" section.
    • Open an issue or PR on any number0 repos that are affected by this breaking change. Give guidance on how the updates should be handled or do the actual updates themselves. The major ones are:

@Frando Frando requested review from flub and matheus23 November 10, 2025 14:08
@github-actions
Copy link

github-actions bot commented Nov 10, 2025

Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/3632/docs/iroh/

Last updated: 2025-11-11T16:00:29Z

Copy link
Member

@matheus23 matheus23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Personal preference would be to switch the inner Box from being referenced as .0 to.inner, but that's a minor nit.

@Frando Frando force-pushed the Frando/mp-path-watcher-improve branch from 4846af0 to 8fd336a Compare November 10, 2025 14:24
@n0bot n0bot bot added this to iroh Nov 10, 2025
@github-project-automation github-project-automation bot moved this to 🏗 In progress in iroh Nov 10, 2025
@Frando Frando force-pushed the Frando/mp-path-watcher-improve branch from b5dd4c1 to c70ad9e Compare November 11, 2025 15:58
@Frando
Copy link
Member Author

Frando commented Nov 11, 2025

I'd be in favor of merging this, it is the cleaner design for now. I don't like storing the Watchable in the Connection because this keeps the path watcher alive after the connection closed if there is still a strong clone of Connection somewhere.

@github-actions
Copy link

Netsim report & logs for this PR have been generated and is available at: LOGS
This report will remain available for 3 days.

Last updated for commit: 02c09e2

Copy link
Contributor

@flub flub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, LGTM. I was just wondering if that ARC was possible.

@Frando Frando merged commit 492b74e into feat-multipath Nov 11, 2025
17 of 28 checks passed
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in iroh Nov 11, 2025
@matheus23 matheus23 deleted the Frando/mp-path-watcher-improve branch November 11, 2025 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

4 participants