Conversation
|
Note: The syntax might be cleaner using the select macro: https://rust-lang.github.io/async-book/06_multiple_futures/03_select.html |
|
Just an FYI that I created a crate with CancelationToken. Again, looking for some feedback! |
yoshuawuyts
left a comment
yoshuawuyts
left a comment
There was a problem hiding this comment.
Hey @GWBasic, thanks so much for kicking this off!
In broad strokes it seems what you're implementing is in the right direction; but we could probably use a round of design feedback on this. Usually I'd suggest opening an issue first, but given we're already discussing this in a PR, doing that here seems fine.
Specifically I want to get a better idea of how we expect end-users to register cancellation in Tide, and how that will work with the various listeners. cc/ @jbr I think you may have had a few ideas here?
| async-std = { version = "1.6.5", features = ["unstable"] } | ||
| async-trait = "0.1.41" | ||
| femme = { version = "2.1.1", optional = true } | ||
| futures = "0.3.7" |
There was a problem hiding this comment.
We probably don't want to depend on futures directly since it adds about a minute of compile time in certain setups. Instead it's preferable to use smaller crates and/or async-std directly.
There was a problem hiding this comment.
Futures is required for "select": https://rust-lang.github.io/async-book/06_multiple_futures/03_select.html
Honestly, I don't see a way to do this without "select," otherwise you end up waiting for a final incoming http request before ending the loop accepting incoming requests. (See my comment about when I reviewed stop-token.)
Maybe it's worth trying to implement something like select manually, but, IMO, that might be a fools' errand.
There was a problem hiding this comment.
I think what we would prefer is futures_lite::future::race.
There was a problem hiding this comment.
Great advice!
Let me see what I can do. It'll probably be a few days before I can update this PR.
| shared_state: self.shared_state.clone() | ||
| } | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
The stop-token does much the same, and we're looking to integrate it into async-std in the near future. If possible it'd be great to use that crate directly instead of defining our own here.
There was a problem hiding this comment.
One thing I like about stop-token is that it supports streams.
However, the authors describe it as "experimental." Upon closer inspection:
- The documentation doesn't build: https://docs.rs/crate/stop-token/0.1.2
- The tests are limited: https://github.com/async-rs/stop-token/blob/master/tests/tests.rs
- Based on how I interpret https://github.com/async-rs/stop-token/blob/master/src/lib.rs#L170, canceling via stop token cancels the next read from the stream. (IE, if I cancel NOW, the background task doesn't cancel until there's another item in the stream.)
I should point out that the library that I made with cancelation token has full documentation and tests: https://crates.io/crates/sync-tokens
|
@yoshuawuyts : Regarding how end users can register for cancelation, take a look at the example code I published here: https://docs.rs/sync-tokens/0.1.0/sync_tokens/ Pay attention to main:
|
|
Replaced with #766 due to excessive merge conflicts |
3 participants
Fixes #528
It works by awaiting on two futures and returns when the first completes: https://github.com/http-rs/tide/compare/main...GWBasic:GracefulShutdown2?w=1#diff-1f14b8b6953e2388b19efaf814862a89c0b57c45a6814f79ed373fde05d864d0R82
I then created a "CancelationToken" future that returns once it is canceled.
(For context, see #528 (comment))