Skip to content

Commit 870c76e

Browse files
authored
feat: Make Endpoint::close infallible (#3112)
## Description <!-- A summary of what this pull request achieves and a rough list of changes. --> The only source for a `Result::Err` in `Endpoint::close` was a failed magic sock actor `Shutdown` message send. This send should basically shutdown the actor, and sending only fails when the actor dropped the receiver, which only happens when the actor already shut down, gracefully or otherwise. In any case, it can't react to that anyways and it's not worth surfacing that error to the user. ## Breaking Changes <!-- Optional, if there are any breaking changes document them, including how to migrate older code. --> - `iroh::Endpoint::close`'s future is now infallible, instead of returning a `Result`. ## Change checklist - [x] Self-review. - [x] 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.~~ - [x] All breaking changes documented.
1 parent 1ae820d commit 870c76e

File tree

8 files changed

+25
-32
lines changed

8 files changed

+25
-32
lines changed

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ let response = recv.read_to_end(1000).await?;
7777
assert_eq!(&response, b"Hello, world!");
7878

7979
// Close the endpoint and all its connections
80-
endpoint.close().await?;
80+
endpoint.close().await;
8181
```
8282

8383
And on the accepting side:

iroh-relay/src/server/clients.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ impl Clients {
7171
/// peer is gone from the network.
7272
///
7373
/// Must be passed a matching connection_id.
74-
pub(super) async fn unregister<'a>(&self, connection_id: u64, node_id: NodeId) {
74+
pub(super) async fn unregister(&self, connection_id: u64, node_id: NodeId) {
7575
trace!(
7676
node_id = node_id.fmt_short(),
7777
connection_id,

iroh/bench/src/lib.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,17 +83,16 @@ pub enum EndpointSelector {
8383
}
8484

8585
impl EndpointSelector {
86-
pub async fn close(self) -> Result<()> {
86+
pub async fn close(self) {
8787
match self {
8888
EndpointSelector::Iroh(endpoint) => {
89-
endpoint.close().await?;
89+
endpoint.close().await;
9090
}
9191
#[cfg(not(any(target_os = "freebsd", target_os = "openbsd", target_os = "netbsd")))]
9292
EndpointSelector::Quinn(endpoint) => {
9393
endpoint.close(0u32.into(), b"");
9494
}
9595
}
96-
Ok(())
9796
}
9897
}
9998

@@ -255,7 +254,7 @@ pub async fn client_handler(
255254
// to `Arc`ing them
256255
connection.close(0u32, b"Benchmark done");
257256

258-
endpoint.close().await?;
257+
endpoint.close().await;
259258

260259
if opt.stats {
261260
println!("\nClient connection stats:\n{:#?}", connection.stats());

iroh/examples/connect.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,6 @@ async fn main() -> anyhow::Result<()> {
9191

9292
// We received the last message: close all connections and allow for the close
9393
// message to be sent.
94-
endpoint.close().await?;
94+
endpoint.close().await;
9595
Ok(())
9696
}

iroh/examples/transfer.rs

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -218,13 +218,7 @@ async fn fetch(ticket: &str, relay_url: Option<String>, relay_only: bool) -> any
218218

219219
// We received the last message: close all connections and allow for the close
220220
// message to be sent.
221-
tokio::time::timeout(Duration::from_secs(3), async move {
222-
let res = endpoint.close().await;
223-
if res.is_err() {
224-
println!("failed to close connection: {res:#?}");
225-
}
226-
})
227-
.await?;
221+
tokio::time::timeout(Duration::from_secs(3), endpoint.close()).await?;
228222

229223
let duration = start.elapsed();
230224
println!(

iroh/src/endpoint.rs

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -972,21 +972,17 @@ impl Endpoint {
972972
/// Be aware however that the underlying UDP sockets are only closed
973973
/// on [`Drop`], bearing in mind the [`Endpoint`] is only dropped once all the clones
974974
/// are dropped.
975-
///
976-
/// Returns an error if closing the magic socket failed.
977-
/// TODO: Document error cases.
978-
pub async fn close(&self) -> Result<()> {
975+
pub async fn close(&self) {
979976
if self.is_closed() {
980-
return Ok(());
977+
return;
981978
}
982979

983980
tracing::debug!("Closing connections");
984981
self.endpoint.close(0u16.into(), b"");
985982
self.endpoint.wait_idle().await;
986983

987984
tracing::debug!("Connections closed");
988-
self.msock.close().await?;
989-
Ok(())
985+
self.msock.close().await;
990986
}
991987

992988
/// Check if this endpoint is still alive, or already closed.
@@ -1594,7 +1590,7 @@ mod tests {
15941590

15951591
info!("closing endpoint");
15961592
// close the endpoint and restart it
1597-
endpoint.close().await.unwrap();
1593+
endpoint.close().await;
15981594

15991595
info!("restarting endpoint");
16001596
// now restart it and check the addressing info of the peer
@@ -1693,7 +1689,7 @@ mod tests {
16931689
send.stopped().await.unwrap();
16941690
recv.read_to_end(0).await.unwrap();
16951691
info!("client finished");
1696-
ep.close().await.unwrap();
1692+
ep.close().await;
16971693
info!("client closed");
16981694
}
16991695
.instrument(error_span!("client", %i))

iroh/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@
173173
//!
174174
//! // Gracefully close the connection and endpoint.
175175
//! conn.close(1u8.into(), b"done");
176-
//! ep.close().await?;
176+
//! ep.close().await;
177177
//! println!("Client closed");
178178
//! Ok(())
179179
//! }
@@ -202,7 +202,7 @@
202202
//!
203203
//! // Wait for the client to close the connection and gracefully close the endpoint.
204204
//! conn.closed().await;
205-
//! ep.close().await?;
205+
//! ep.close().await;
206206
//! Ok(())
207207
//! }
208208
//! ```

iroh/src/magicsock.rs

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1654,12 +1654,18 @@ impl Handle {
16541654
/// Polling the socket ([`AsyncUdpSocket::poll_recv`]) will return [`Poll::Pending`]
16551655
/// indefinitely after this call.
16561656
#[instrument(skip_all, fields(me = %self.msock.me))]
1657-
pub(crate) async fn close(&self) -> Result<()> {
1657+
pub(crate) async fn close(&self) {
16581658
if self.msock.is_closed() {
1659-
return Ok(());
1659+
return;
16601660
}
16611661
self.msock.closing.store(true, Ordering::Relaxed);
1662-
self.msock.actor_sender.send(ActorMessage::Shutdown).await?;
1662+
// If this fails, then there's no receiver listening for shutdown messages,
1663+
// so nothing to shut down anyways.
1664+
self.msock
1665+
.actor_sender
1666+
.send(ActorMessage::Shutdown)
1667+
.await
1668+
.ok();
16631669
self.msock.closed.store(true, Ordering::SeqCst);
16641670

16651671
let mut tasks = self.actor_tasks.lock().await;
@@ -1681,8 +1687,6 @@ impl Handle {
16811687
debug!("aborting remaining {}/3 tasks", tasks.len());
16821688
tasks.shutdown().await;
16831689
}
1684-
1685-
Ok(())
16861690
}
16871691
}
16881692

@@ -3408,8 +3412,8 @@ mod tests {
34083412
println!("closing endpoints");
34093413
let msock1 = m1.endpoint.magic_sock();
34103414
let msock2 = m2.endpoint.magic_sock();
3411-
m1.endpoint.close().await?;
3412-
m2.endpoint.close().await?;
3415+
m1.endpoint.close().await;
3416+
m2.endpoint.close().await;
34133417

34143418
assert!(msock1.msock.is_closed());
34153419
assert!(msock2.msock.is_closed());

0 commit comments

Comments
 (0)