-
Notifications
You must be signed in to change notification settings - Fork 315
Commit e0f10ce
authored
refactor(multipath): Make registering connections with the magicsock async (#3629)
## Description
Currently, `Magicsock::register_connection` is a sync function, but
needs to send over an async channel to notify the endpoint state actor
about the new connection. It currently employs a hack to achieve that:
it spawns a tokio task for sending the message.
This PR cleans this up by making `regsiter_connection` return a future,
and awaits this future at the various sites where we go from
quinn::Connection to iroh Connection. Luckily, all these call sites
already are in async contexts.
* When going from `Connecting` or `Accepting` to `Connection`, we await
the registration after having the `quinn::Connecting` completes. The
future is stored in an option instead of using a state enum as you would
usually, because we need unconditional access to the `quinn::Connecting`
in the functions on `Connecting`/`Accepting`.
* For the `(Incoming|Outgoing)ZeroRttConnection`, we store a future that
first awaits the handshake and then registers the connection. So we need
only a single future here.
With `register_connection` being async, we can also clean up some of the
not-so-nice things introduced in #3622: Because we now have an async
function, we can let the endpoint state actor return a reply. This makes
it much more straightforward because we can have the endpoint state
actor initialize a watcher for the paths and return it instead of having
to do a weird dance with parts of the state being initialized or stored
outside of the endpoint state actor to satisfy the sync function
constraints. This is much nicer now IMO.
## Breaking Changes
<!-- Optional, if there are any breaking changes document them,
including how to migrate older code. -->
## Notes & open questions
This adds a boxed future into the process of going from a `Connecting`
to a `Connection`. If we really wanted, we could use a manually
implemented future instead. However, I don't think one boxed future *per
connection* is an issue, so I'd prefer to leave it like this
(implementing a manual future for `tokio::mpsc::Sender::send` is
cumbersome).
## Change checklist
<!-- Remove any that are not relevant. -->
- [ ] Self-review.
- [ ] Documentation updates following the [style
guide](https://rust-lang.github.io/rfcs/1574-more-api-documentation-conventions.html#appendix-a-full-conventions-text),
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:
- [ ] [`quic-rpc`](https://github.com/n0-computer/quic-rpc)
- [ ] [`iroh-gossip`](https://github.com/n0-computer/iroh-gossip)
- [ ] [`iroh-blobs`](https://github.com/n0-computer/iroh-blobs)
- [ ] [`dumbpipe`](https://github.com/n0-computer/dumbpipe)
- [ ] [`sendme`](https://github.com/n0-computer/sendme)1 parent 1d5937c commit e0f10ceCopy full SHA for e0f10ce
File tree
Expand file treeCollapse file tree
4 files changed
+163
-246
lines changedOpen diff view settings
Filter options
- iroh/src
- endpoint
- magicsock
- endpoint_map
Expand file treeCollapse file tree
4 files changed
+163
-246
lines changedOpen diff view settings
0 commit comments