refactor(iroh): Remove lock around quinn::Endpoint again#3941
Merged
ramfox merged 4 commits intoramfox/dropped-endpointfrom Feb 14, 2026
Merged
refactor(iroh): Remove lock around quinn::Endpoint again#3941ramfox merged 4 commits intoramfox/dropped-endpointfrom
ramfox merged 4 commits intoramfox/dropped-endpointfrom
Conversation
3 tasks
|
Documentation for this PR has been generated and is available at: https://n0-computer.github.io/iroh/pr/3941/docs/iroh/ Last updated: 2026-02-13T23:58:37Z |
11 tasks
## Description Based on #3941 which is based on #3879 * Add `Endpoint::closed`, which returns a `CancellationToken` that is cancelled once the endpoint is closing * Document watcher behavior with regards to closing the endpoint * Fix test ## Breaking Changes <!-- Optional, if there are any breaking changes document them, including how to migrate older code. --> ## Notes & open questions * Instead of returning a `CancellationToken` we could make this an `async fn`, however to me it looks quite useful to get a `CancellationToken` and be able to use `tokio::spawn(endpoint_closed.run_until_cancelled_owned(async move { .. })))` * The cancellation token is cancelled when `Endpoint::close` *starts*, not when it *ends*. I wasn't sure what expected behavior is here. I opted for start because this guarantees that the endpoint is fully usable until then.
ramfox
pushed a commit
that referenced
this pull request
Feb 14, 2026
## Description Based on #3879 Depends on n0-computer/noq#426 With n0-computer/noq#426 the quinn::EndpointDriver stops once the quinn::Endpoint is closed and all connections are drained. Thus we don't need the `RwLock` around the `quinn::Endpoint` anymore. See [this thread](#3879 (comment)) for details. * Add `Endpoint::closed`, which returns a `CancellationToken` that is cancelled once the endpoint is closing * Document watcher behavior with regards to closing the endpoint * Fix test ## Notes & open questions * Instead of returning a `CancellationToken` we could make this an `async fn`, however to me it looks quite useful to get a `CancellationToken` and be able to use `tokio::spawn(endpoint_closed.run_until_cancelled_owned(async move { .. })))` * The cancellation token is cancelled when `Endpoint::close` *starts*, not when it *ends*. I wasn't sure what expected behavior is here. I opted for start because this guarantees that the endpoint is fully usable until then.
ramfox
pushed a commit
that referenced
this pull request
Feb 17, 2026
## Description Based on #3879 Depends on n0-computer/noq#426 With n0-computer/noq#426 the quinn::EndpointDriver stops once the quinn::Endpoint is closed and all connections are drained. Thus we don't need the `RwLock` around the `quinn::Endpoint` anymore. See [this thread](#3879 (comment)) for details. * Add `Endpoint::closed`, which returns a `CancellationToken` that is cancelled once the endpoint is closing * Document watcher behavior with regards to closing the endpoint * Fix test ## Notes & open questions * Instead of returning a `CancellationToken` we could make this an `async fn`, however to me it looks quite useful to get a `CancellationToken` and be able to use `tokio::spawn(endpoint_closed.run_until_cancelled_owned(async move { .. })))` * The cancellation token is cancelled when `Endpoint::close` *starts*, not when it *ends*. I wasn't sure what expected behavior is here. I opted for start because this guarantees that the endpoint is fully usable until then.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Based on #3879
Depends on n0-computer/noq#426
With n0-computer/noq#426 the quinn::EndpointDriver stops once the quinn::Endpoint is closed and all connections are drained. Thus we don't need the
RwLockaround thequinn::Endpointanymore.See this thread for details.
Somehow,
Endpoint::watch_addrs().stream()now hangs again, couldn't spot yet how that interacts, weird.Edit: I now know why the
watch_addrs()watcher doesn't stop: It contains alocal_addrswatcher, which is a watcher onto a watchable owned byTransportswhich is owned by thequinn::Endpoint, which with this change is not dropped at this point.Breaking Changes
Notes & open questions
Change checklist
quic-rpciroh-gossipiroh-blobsdumbpipesendme