-
Notifications
You must be signed in to change notification settings - Fork 321
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
Graceful shutdown2 #746
Graceful shutdown2 #746
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,4 +5,5 @@ dist/ | |
npm-debug.log* | ||
Cargo.lock | ||
.DS_Store | ||
.idea | ||
.idea | ||
.vscode/launch.json |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
use std::future::Future; | ||
use std::pin::Pin; | ||
use std::sync::{Arc, Mutex}; | ||
use std::task::{Context, Poll, Waker}; | ||
|
||
#[derive(Debug)] | ||
pub struct CancelationToken { | ||
shared_state: Arc<Mutex<CancelationTokenState>> | ||
} | ||
|
||
#[derive(Debug)] | ||
struct CancelationTokenState { | ||
canceled: bool, | ||
waker: Option<Waker> | ||
} | ||
|
||
/// Future that allows gracefully shutting down the server | ||
impl CancelationToken { | ||
pub fn new() -> CancelationToken { | ||
CancelationToken { | ||
shared_state: Arc::new(Mutex::new(CancelationTokenState { | ||
canceled: false, | ||
waker: None | ||
})) | ||
} | ||
} | ||
|
||
/// Call to shut down the server | ||
pub fn complete(&self) { | ||
let mut shared_state = self.shared_state.lock().unwrap(); | ||
|
||
shared_state.canceled = true; | ||
if let Some(waker) = shared_state.waker.take() { | ||
waker.wake() | ||
} | ||
} | ||
} | ||
|
||
impl Future for CancelationToken { | ||
type Output = (); | ||
|
||
fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> { | ||
let mut shared_state = self.shared_state.lock().unwrap(); | ||
|
||
if shared_state.canceled { | ||
Poll::Ready(()) | ||
} else { | ||
shared_state.waker = Some(cx.waker().clone()); | ||
Poll::Pending | ||
} | ||
} | ||
} | ||
|
||
impl Clone for CancelationToken { | ||
fn clone(&self) -> Self { | ||
CancelationToken { | ||
shared_state: self.shared_state.clone() | ||
} | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One thing I like about stop-token is that it supports streams. However, the authors describe it as "experimental." Upon closer inspection:
I should point out that the library that I made with cancelation token has full documentation and tests: https://crates.io/crates/sync-tokens |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think what we would prefer is
futures_lite::future::race
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great advice!
Let me see what I can do. It'll probably be a few days before I can update this PR.