Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

consider making Endpoint::cancel_token public #3096

Closed
arilotter opened this issue Jan 3, 2025 · 0 comments · Fixed by #3101
Closed

consider making Endpoint::cancel_token public #3096

arilotter opened this issue Jan 3, 2025 · 0 comments · Fixed by #3101

Comments

@arilotter
Copy link

pub(crate) fn cancel_token(&self) -> &CancellationToken {

The cancel_token method in Endpoint is pub(crate) only.
It's used inside the Router to automatically shut down if the Endpoint is shut down, but this is a privilege uniquely granted to Router.

// We use a child token of the endpoint, to ensure that this is shutdown
// when the endpoint is shutdown, but that we can shutdown ourselves independently.
let cancel = endpoint.cancel_token().child_token();

End-user code with a custom accept loop is not able to see this cancel_token method, and thus is not able to automatically shut down should the Endpoint shut down.

I request that the cancel_token() method be made pub, so my end-user code can have the same behavior as Router and cleanly auto-shutdown.

@Arqu Arqu added this to iroh Jan 3, 2025
arilotter added a commit to arilotter/iroh that referenced this issue Jan 6, 2025
It's used inside the Router to automatically shut down if the Endpoint is shut down, but this is a privilege uniquely granted to Router since it's part of Iroh proper.

End-user code with a custom accept loop is not able to see this cancel_token method, and thus is not able to automatically shut down should the Endpoint shut down.

To bring end-user accept loops into feature parity with Iroh's internals, this method must be `pub`.

Closes n0-computer#3096
flub added a commit that referenced this issue Jan 7, 2025
The internal CancellationToke was used to know by other parts of the
code when the endpoint is shut down.  But those bits of code already
have mechanisms to do so.  This bit of API makes is a bit of extra
complexity that is not needed.

Closes #3096
github-merge-queue bot pushed a commit that referenced this issue Jan 8, 2025
## Description

The internal CancellationToke was used to know by other parts of the
code when the endpoint is shut down.  But those bits of code already
have mechanisms to do so.  This bit of API makes is a bit of extra
complexity that is not needed.

## Breaking Changes

None, this is internal.

## Notes & open questions

Closes #3096.
Closes #3098 (replaces).

Maybe not directly but now there's an example of how to write an
accept loop without having to rely on the CancellationToken.


## 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.
@flub flub closed this as completed in #3101 Jan 8, 2025
@github-project-automation github-project-automation bot moved this to ✅ Done in iroh Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
1 participant