Repository: static-web-server/static-web-server (language: rust)

Issue:
Allow waiting for requests to finish before shutting down
**Is your feature request related to a problem? Please describe.**
I'm experimenting with running `static-web-server` on Kubernetes. The issue I'm having is, whenever I deploy a new version of the container running `static-web-server`, some of the HTTP requests made during the deployment are failing. Peeking through the code, it seems that the server doesn't wait for existing requests to complete when it receives the termination signal. I suspect that this is what's causing the issue.

**Describe the solution you'd like**
I'd be nice if the server waited for the existing requests to finish before shutting down. If this behavior is not desirable as default, it could be toggled with some configuration option (e.g. `SERVER_GRACEFUL_SHUTDOWN`, or maybe `SERVER_SHUTDOWN_TIMEOUT`).

**Describe alternatives you've considered**
I was able to workaround the issue by adding a 5 seconds delay before the TERM signal is actually sent to the server. It seems to work, but feels hackish.



Discussion / hints:
> Peeking through the code, it seems that the server doesn't wait for existing requests to complete when it receives the termination signal. I suspect that this is what's causing the issue.

You're right! In fact is a missing feature.
Yeah, we need to revisit the _gracefull shutdown_ of our internal server implementation (Hyper) for the `v2` (what I assume you are using).

But in general is not a big deal. Even we can add it by default since it should be more reasonable than a new option or env.
> ...`v2` (what I assume you are using).

Yes, I'm using the `v2` (through the `joseluisq/static-web-server:2` Docker image).

> Even we can add it by default since it should be more reasonable than a new option or env.

Yeah, that's the behavior I'd want 99% of the times, so it makes sense to me that it would be the default.

Fix patch:
diff --git a/Cargo.lock b/Cargo.lock
index c17be3b5..0d0b2f45 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -202,16 +202,6 @@ dependencies = [
  "cfg-if",
 ]
 
-[[package]]
-name = "ctrlc"
-version = "3.2.1"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a19c6cedffdc8c03a3346d723eb20bd85a13362bb96dc2ac000842c6381ec7bf"
-dependencies = [
- "nix",
- "winapi",
-]
-
 [[package]]
 name = "digest"
 version = "0.9.0"
@@ -546,15 +536,6 @@ version = "2.4.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "308cc39be01b73d0d18f82a0e7b2a3df85245f84af96fdddc5d202d27e47b86a"
 
-[[package]]
-name = "memoffset"
-version = "0.6.4"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "59accc507f1338036a0477ef61afdae33cde60840f4dfe481319ce3ad116ddf9"
-dependencies = [
- "autocfg",
-]
-
 [[package]]
 name = "mime"
 version = "0.3.16"
@@ -603,19 +584,6 @@ dependencies = [
  "winapi",
 ]
 
-[[package]]
-name = "nix"
-version = "0.23.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f305c2c2e4c39a82f7bf0bf65fb557f9070ce06781d4f2454295cc34b1c43188"
-dependencies = [
- "bitflags",
- "cc",
- "cfg-if",
- "libc",
- "memoffset",
-]
-
 [[package]]
 name = "ntapi"
 version = "0.3.6"
@@ -853,6 +821,38 @@ dependencies = [
  "lazy_static",
 ]
 
+[[package]]
+name = "signal-hook"
+version = "0.3.10"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "9c98891d737e271a2954825ef19e46bd16bdb98e2746f2eec4f7a4ef7946efd1"
+dependencies = [
+ "cc",
+ "libc",
+ "signal-hook-registry",
+]
+
+[[package]]
+name = "signal-hook-registry"
+version = "1.4.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e51e73328dc4ac0c7ccbda3a494dfa03df1de2f46018127f60c693f2648455b0"
+dependencies = [
+ "libc",
+]
+
+[[package]]
+name = "signal-hook-tokio"
+version = "0.3.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f6c5d32165ff8b94e68e7b3bdecb1b082e958c22434b363482cfb89dcd6f3ff8"
+dependencies = [
+ "futures-core",
+ "libc",
+ "signal-hook",
+ "tokio",
+]
+
 [[package]]
 name = "slab"
 version = "0.4.5"
@@ -889,7 +889,6 @@ dependencies = [
  "async-compression",
  "bcrypt",
  "bytes",
- "ctrlc",
  "futures-util",
  "headers",
  "http",
@@ -901,6 +900,8 @@ dependencies = [
  "num_cpus",
  "percent-encoding",
  "pin-project",
+ "signal-hook",
+ "signal-hook-tokio",
  "structopt",
  "time",
  "tokio",
@@ -985,7 +986,9 @@ dependencies = [
  "memchr",
  "mio",
  "num_cpus",
+ "once_cell",
  "pin-project-lite",
+ "signal-hook-registry",
  "tokio-macros",
  "winapi",
 ]
diff --git a/Cargo.toml b/Cargo.toml
index f99ac8af..273dbba4 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -32,7 +32,6 @@ anyhow = "1.0"
 async-compression = { version = "0.3", features = ["brotli", "deflate", "gzip", "tokio"] }
 bcrypt = "0.10"
 bytes = "1.0"
-ctrlc = { version = "3.1", features = ["termination"] }
 futures-util = { version = "0.3", default-features = false, features = ["sink"] }
 headers = { git = "https://github.com/joseluisq/hyper-headers.git", branch = "headers_encoding" }
 http = "0.2"
@@ -45,7 +44,7 @@ percent-encoding = "2.1"
 pin-project = "1.0"
 structopt = { version = "0.3", default-features = false }
 time = "0.1"
-tokio = { version = "1", features = ["rt-multi-thread", "macros", "fs", "io-util"], default-features = false }
+tokio = { version = "1", features = ["rt-multi-thread", "macros", "fs", "io-util", "signal"], default-features = false }
 tokio-rustls = { version = "0.22" }
 tokio-util = { version = "0.6", features = ["io"] }
 tracing = "0.1"
@@ -54,6 +53,10 @@ tracing-subscriber = "0.2"
 [target.'cfg(all(target_env = "musl", target_pointer_width = "64"))'.dependencies.jemallocator]
 version = "0.3"
 
+[target.'cfg(not(windows))'.dependencies]
+signal-hook = { version = "0.3.4", features = ["extended-siginfo"] }
+signal-hook-tokio = { version = "0.3", features = ["futures-v0_3"], default-features = false }
+
 [dev-dependencies]
 bytes = "1.0"
 
diff --git a/docker/alpine/Dockerfile b/docker/alpine/Dockerfile
index f8e4708e..b0e23b43 100644
--- a/docker/alpine/Dockerfile
+++ b/docker/alpine/Dockerfile
@@ -19,7 +19,11 @@ COPY ./docker/alpine/entrypoint.sh /
 COPY ./docker/public /public
 
 EXPOSE 80
+
+STOPSIGNAL SIGQUIT
+
 ENTRYPOINT ["/entrypoint.sh"]
+
 CMD ["static-web-server"]
 
 # Metadata
diff --git a/docker/scratch/Dockerfile b/docker/scratch/Dockerfile
index 2124c104..e89f8a80 100644
--- a/docker/scratch/Dockerfile
+++ b/docker/scratch/Dockerfile
@@ -15,6 +15,9 @@ COPY --from=latest /usr/local/bin/static-web-server /
 COPY ./docker/public /public
 
 EXPOSE 80
+
+STOPSIGNAL SIGQUIT
+
 ENTRYPOINT ["/static-web-server"]
 
 # Metadata
diff --git a/src/server.rs b/src/server.rs
index 211de99d..191b3eb6 100644
--- a/src/server.rs
+++ b/src/server.rs
@@ -132,7 +132,7 @@ impl Server {
             },
         });
 
-        // Spawn a new Tokio asynchronous task with its given options
+        // Run the corresponding HTTP Server asynchronously with its given options
 
         if opts.http2 {
             // HTTP/2 + TLS
@@ -140,54 +140,82 @@ impl Server {
             let cert_path = opts.http2_tls_cert.clone();
             let key_path = opts.http2_tls_key.clone();
 
-            tokio::task::spawn(async move {
-                tcp_listener
-                    .set_nonblocking(true)
-                    .expect("cannot set non-blocking");
-                let listener = tokio::net::TcpListener::from_std(tcp_listener)
-                    .expect("failed to create tokio::net::TcpListener");
-                let mut incoming = AddrIncoming::from_listener(listener)?;
-                incoming.set_nodelay(true);
-
-                let tls = TlsConfigBuilder::new()
-                    .cert_path(cert_path)
-                    .key_path(key_path)
-                    .build()
-                    .expect(
-                        "error during TLS server initialization, probably cert or key file missing",
-                    );
-
-                let server =
-                    HyperServer::builder(TlsAcceptor::new(tls, incoming)).serve(router_service);
-
-                tracing::info!(
-                    parent: tracing::info_span!("Server::start_server", ?addr_str, ?threads),
-                    "listening on https://{}",
-                    addr_str
+            tcp_listener
+                .set_nonblocking(true)
+                .expect("cannot set non-blocking");
+            let listener = tokio::net::TcpListener::from_std(tcp_listener)
+                .expect("failed to create tokio::net::TcpListener");
+            let mut incoming = AddrIncoming::from_listener(listener)?;
+            incoming.set_nodelay(true);
+
+            let tls = TlsConfigBuilder::new()
+                .cert_path(cert_path)
+                .key_path(key_path)
+                .build()
+                .expect(
+                    "error during TLS server initialization, probably cert or key file missing",
                 );
 
-                server.await
-            });
+            #[cfg(not(windows))]
+            let signals = signals::create_signals()?;
+            #[cfg(not(windows))]
+            let handle = signals.handle();
+
+            let server =
+                HyperServer::builder(TlsAcceptor::new(tls, incoming)).serve(router_service);
+
+            #[cfg(not(windows))]
+            let server = server.with_graceful_shutdown(signals::wait_for_signals(signals));
+            #[cfg(windows)]
+            let server = server.with_graceful_shutdown(signals::wait_for_ctrl_c());
+
+            tracing::info!(
+                parent: tracing::info_span!("Server::start_server", ?addr_str, ?threads),
+                "listening on https://{}",
+                addr_str
+            );
+
+            tracing::info!("press ctrl+c to shut down the server");
+
+            server.await?;
+
+            #[cfg(not(windows))]
+            handle.close();
         } else {
             // HTTP/1
 
-            tokio::task::spawn(async move {
-                let server = HyperServer::from_tcp(tcp_listener)
-                    .unwrap()
-                    .tcp_nodelay(true)
-                    .serve(router_service);
+            #[cfg(not(windows))]
+            let signals = signals::create_signals()?;
+            #[cfg(not(windows))]
+            let handle = signals.handle();
 
-                tracing::info!(
-                    parent: tracing::info_span!("Server::start_server", ?addr_str, ?threads),
-                    "listening on http://{}",
-                    addr_str
-                );
+            let server = HyperServer::from_tcp(tcp_listener)
+                .unwrap()
+                .tcp_nodelay(true)
+                .serve(router_service);
 
-                server.await
-            });
+            #[cfg(not(windows))]
+            let server = server.with_graceful_shutdown(signals::wait_for_signals(signals));
+            #[cfg(windows)]
+            let server = server.with_graceful_shutdown(signals::wait_for_ctrl_c());
+
+            tracing::info!(
+                parent: tracing::info_span!("Server::start_server", ?addr_str, ?threads),
+                "listening on http://{}",
+                addr_str
+            );
+
+            tracing::info!("press ctrl+c to shut down the server");
+
+            server.await?;
+
+            #[cfg(not(windows))]
+            handle.close();
         }
 
-        signals::wait_for_ctrl_c()
+        tracing::warn!("termination signal caught, shutting down the server execution");
+
+        Ok(())
     }
 }
 
diff --git a/src/signals.rs b/src/signals.rs
index 49941dd2..20fc4da5 100644
--- a/src/signals.rs
+++ b/src/signals.rs
@@ -1,21 +1,38 @@
-use ctrlc;
-use std::sync::mpsc::channel;
+#[cfg(not(windows))]
+use {
+    crate::Result, futures_util::stream::StreamExt, signal_hook::consts::signal::*,
+    signal_hook_tokio::Signals,
+};
 
-use crate::{Context, Result};
-
-/// It waits for a `Ctrl-C` incoming signal.
-pub fn wait_for_ctrl_c() -> Result {
-    let (tx, rx) = channel();
-
-    ctrlc::set_handler(move || tx.send(()).expect("could not send signal on channel"))
-        .with_context(|| "error setting Ctrl-C handler".to_owned())?;
-
-    tracing::info!("press Ctrl+C to shutdown server");
-
-    rx.recv()
-        .with_context(|| "could not receive signal from channel".to_owned())?;
+#[cfg(not(windows))]
+/// It creates a common list of signals stream for `SIGTERM`, `SIGINT` and `SIGQUIT` to be observed.
+pub fn create_signals() -> Result<Signals> {
+    Ok(Signals::new(&[SIGHUP, SIGTERM, SIGINT, SIGQUIT])?)
+}
 
-    tracing::warn!("Ctrl+C signal caught, shutting down server execution");
+#[cfg(not(windows))]
+/// It waits for a specific type of incoming signals included `ctrl+c`.
+pub async fn wait_for_signals(signals: Signals) {
+    let mut signals = signals.fuse();
+    while let Some(signal) = signals.next().await {
+        match signal {
+            SIGHUP => {
+                // Note: for now we don't do something for SIGHUPs
+                tracing::debug!("SIGHUP caught, nothing to do about")
+            }
+            SIGTERM | SIGINT | SIGQUIT => {
+                tracing::debug!("an incoming SIGTERM received, SIGINT or SIGQUIT signal, delegating graceful shutdown to server");
+                break;
+            }
+            _ => unreachable!(),
+        }
+    }
+}
 
-    Ok(())
+#[cfg(windows)]
+/// It waits for an incoming `ctrl+c` signal on Windows.
+pub async fn wait_for_ctrl_c() {
+    tokio::signal::ctrl_c()
+        .await
+        .expect("failed to install ctrl+c signal handler");
 }


Test patch: