Skip to content

Commit

Permalink
ref(iroh-net)!: Make Endpoint::close not consume self (#2882)
Browse files Browse the repository at this point in the history
## Description

The endpoint can be freely cloned, so consuming `self` on shutdown is
somewhat fruitless. This changes it to take `&self` like a normal
method.

## Breaking Changes

- `Endpoint::close` now takes `&self` rather than `self`. This can in
some situations mean an existing (clone of an) endpoint might be dropped
too early as a temporary variable.

## Notes & open questions

## 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.
- [x] Tests if relevant.
- [x] All breaking changes documented.

---------

Co-authored-by: iacore <noreply+gpg-stub@1a-insec.net>
Co-authored-by: Floris Bruynooghe <flub@devork.be>
Co-authored-by: Floris Bruynooghe <flub@n0.computer>
  • Loading branch information
4 people authored Nov 27, 2024
1 parent cc9e4e6 commit 50f66dd
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 7 deletions.
6 changes: 2 additions & 4 deletions iroh-net/src/endpoint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -964,7 +964,7 @@ impl Endpoint {
///
/// Returns an error if closing the magic socket failed.
/// TODO: Document error cases.
pub async fn close(self, error_code: VarInt, reason: &[u8]) -> Result<()> {
pub async fn close(&self, error_code: VarInt, reason: &[u8]) -> Result<()> {
let Endpoint {
msock,
endpoint,
Expand All @@ -975,9 +975,7 @@ impl Endpoint {
tracing::debug!("Closing connections");
endpoint.close(error_code, reason);
endpoint.wait_idle().await;
// In case this is the last clone of `Endpoint`, dropping the `quinn::Endpoint` will
// make it more likely that the underlying socket is not polled by quinn anymore after this
drop(endpoint);

tracing::debug!("Connections closed");

msock.close().await?;
Expand Down
4 changes: 1 addition & 3 deletions iroh-router/src/router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,7 @@ async fn shutdown(endpoint: &Endpoint, protocols: Arc<ProtocolMap>) {
// Closing the Endpoint is the equivalent of calling Connection::close on all
// connections: Operations will immediately fail with ConnectionError::LocallyClosed.
// All streams are interrupted, this is not graceful.
endpoint
.clone()
.close(error_code.into(), b"provider terminating"),
endpoint.close(error_code.into(), b"provider terminating"),
// Shutdown protocol handlers.
protocols.shutdown(),
);
Expand Down

0 comments on commit 50f66dd

Please sign in to comment.