-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Handle SIGTERM properly #183
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Li Yi <denverdino@gmail.com>
Sorry for the lack of reply - I've been away. Thanks for this - I'll take a look! |
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.
Couple of things, hopefully minor. Thanks!
@@ -11,13 +11,19 @@ use hyper::{ | |||
use hyper::{Body, Response, Server}; | |||
use tokio::net::TcpStream; | |||
use tokio_rustls::server::TlsStream; | |||
use tokio::signal::unix::SignalKind; |
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.
Will this work on Windows? Might need to be conditionally compiled...
.await?; | ||
let server = Server::builder(tls::TlsHyperAcceptor::new(&self.address, &tls.cert_path, &tls.key_path).await?) | ||
.serve(mk_svc); | ||
let graceful = server.with_graceful_shutdown(shutdown_signal()); |
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 had problems with with_graceful_shutdown
in Spin because it kept the process alive until all connections had closed, which could be a couple of minutes due to keep-alives. See fermyon/spin#73 for the problem and links to relevant Hyper issues, and fermyon/spin#232 for how we solved it there.
Signed-off-by: Li Yi denverdino@gmail.com
Fix #182
Tested in Mac/Linux
In the separated terminal