Fix handling of SIGTERM on *nix (#178)

* Fix handling of SIGTERM on *nix This is just for future extensibility * Fix error codes I have been silly enough to break error codes
skytable · Jun 22, 2021 · d53a0cb · d53a0cb
1 parent 5be3041
commit d53a0cb
Show file tree

Hide file tree

Showing 7 changed files with 62 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,8 @@ All changes in this project will be noted in this file.
 - More than one process can no longer concurrently use the same data directory, preventing any possible data corruption [see [#169](https://github.com/skytable/skytable/pull/169), [#167](https://github.com/skytable/skytable/issues/167)]
 - Fixed longstanding error in `sky-bench` component that caused key collisions
 - Fixed bug in `sky-bench` that allowed passing 0 for the inputs
+- Fixed handling of SIGTERM in `skyd` [see [#178](https://github.com/skytable/skytable/pull/178)]
+- Fixed incorrect termination codes [see [#178](https://github.com/skytable/skytable/pull/178)]
 
 ### Additions
 

diff --git a/cli/src/argparse.rs b/cli/src/argparse.rs
@@ -54,7 +54,7 @@ pub async fn start_repl() {
             Ok(p) => p,
             Err(_) => {
                 eprintln!("ERROR: Invalid port");
-                process::exit(0x100);
+                process::exit(0x01);
             }
         },
         None => 2003,
@@ -63,7 +63,7 @@ pub async fn start_repl() {
         Ok(c) => c,
         Err(e) => {
             eprintln!("ERROR: {}", e);
-            process::exit(0x100);
+            process::exit(0x01);
         }
     };
     let mut runner = Runner::new(con);

diff --git a/libstress/src/traits.rs b/libstress/src/traits.rs
@@ -46,7 +46,7 @@ where
         match self {
             Self::Err(e) => {
                 log::error!("{} : '{}'", msg.to_string(), e);
-                std::process::exit(0x100);
+                std::process::exit(0x01);
             }
             Self::Ok(v) => v,
         }
@@ -61,7 +61,7 @@ impl<T> ExitError<T> for Option<T> {
         match self {
             Self::None => {
                 log::error!("{}", msg.to_string());
-                std::process::exit(0x100);
+                std::process::exit(0x01);
             }
             Self::Some(v) => v,
         }

diff --git a/server/src/config/mod.rs b/server/src/config/mod.rs
@@ -422,7 +422,7 @@ pub fn get_config_file_or_return_cfg() -> Result<ConfigType<ParsedConfig, String
         if let Some(format) = matches.value_of("format") {
             if let Err(e) = compat::upgrade(format) {
                 log::error!("Dataset upgrade failed with error: {}", e);
-                process::exit(0x100);
+                process::exit(0x01);
             } else {
                 process::exit(0x000);
             }

diff --git a/server/src/dbnet/mod.rs b/server/src/dbnet/mod.rs
@@ -246,6 +246,34 @@ impl MultiListener {
     }
 }
 
+#[cfg(unix)]
+use core::{pin::Pin, task::Context, task::Poll};
+#[cfg(unix)]
+use tokio::signal::unix::{signal as fnsignal, Signal, SignalKind};
+#[cfg(unix)]
+/// Object to bind to unix-specific signals
+pub struct UnixTerminationSignal {
+    sigterm: Signal,
+}
+
+#[cfg(unix)]
+impl UnixTerminationSignal {
+    pub fn init() -> Result<Self, String> {
+        let sigterm = fnsignal(SignalKind::terminate())
+            .map_err(|e| format!("Failed to bind to signal with: {}", e))?;
+        Ok(Self { sigterm })
+    }
+}
+
+#[cfg(unix)]
+impl Future for UnixTerminationSignal {
+    type Output = Option<()>;
+
+    fn poll(mut self: Pin<&mut Self>, ctx: &mut Context<'_>) -> Poll<Self::Output> {
+        self.sigterm.poll_recv(ctx)
+    }
+}
+
 /// Start the server waiting for incoming connections or a CTRL+C signal
 pub async fn run(
     ports: PortConfig,
@@ -294,12 +322,28 @@ pub async fn run(
             MultiListener::new_multi(secure_listener, insecure_listener, ssl).await?
         }
     };
-    tokio::select! {
-        _ = server.run_server() => {}
-        _ = sig => {
-            log::info!("Signalling all workers to shut down");
+    #[cfg(not(unix))]
+    {
+        // Non-unix, usually Windows specific signal handling.
+        // FIXME(@ohsayan): For now, let's just
+        // bother with ctrl+c, we'll move ahead as users require them
+        tokio::select! {
+            _ = server.run_server() => {}
+            _ = sig => {}
+        }
+    }
+    #[cfg(unix)]
+    {
+        let sigterm = UnixTerminationSignal::init()?;
+        // apart from CTRLC, the only other thing we care about is SIGTERM
+        // FIXME(@ohsayan): Maybe we should respond to SIGHUP too?
+        tokio::select! {
+            _ = server.run_server() => {},
+            _ = sig => {},
+            _ = sigterm => {}
         }
     }
+    log::info!("Signalling all workers to shut down");
     // drop the signal and let others exit
     drop(signal);
     server.finish_with_termsig().await;

diff --git a/server/src/main.rs b/server/src/main.rs
@@ -108,7 +108,7 @@ fn main() {
             // uh oh, something happened while starting up
             log::error!("{}", e);
             pre_shutdown_cleanup(pid_file);
-            process::exit(0x100);
+            process::exit(1);
         }
     };
     assert_eq!(
@@ -141,7 +141,7 @@ pub fn pre_shutdown_cleanup(pid_file: fs::File) {
     drop(pid_file);
     if let Err(e) = fs::remove_file(PATH) {
         log::error!("Shutdown failure: Failed to remove pid file: {}", e);
-        process::exit(0x100);
+        process::exit(0x01);
     }
 }
 
@@ -168,7 +168,7 @@ fn check_args_and_get_cfg() -> (PortConfig, BGSave, SnapshotConfig, Option<Strin
         }
         Err(e) => {
             log::error!("{}", e);
-            std::process::exit(0x100);
+            std::process::exit(0x01);
         }
     };
     binding_and_cfg
@@ -190,7 +190,7 @@ fn run_pre_startup_tasks() -> fs::File {
             "Startup failure: Another process with parent PID {} is using the data directory",
             pid
         );
-        process::exit(0x100);
+        process::exit(0x01);
     }
     let mut file = match fs::OpenOptions::new()
         .create(true)
@@ -201,12 +201,12 @@ fn run_pre_startup_tasks() -> fs::File {
         Ok(fle) => fle,
         Err(e) => {
             log::error!("Startup failure: Failed to open pid file: {}", e);
-            process::exit(0x100);
+            process::exit(0x01);
         }
     };
     if let Err(e) = file.write_all(process::id().to_string().as_bytes()) {
         log::error!("Startup failure: Failed to write to pid file: {}", e);
-        process::exit(0x100);
+        process::exit(0x01);
     }
     file
 }
diff --git a/sky-bench/src/util.rs b/sky-bench/src/util.rs
@@ -60,7 +60,7 @@ macro_rules! sanity_test {
 macro_rules! err {
     ($note:expr) => {{
         eprintln!("ERROR: {}", $note);
-        std::process::exit(0x100);
+        std::process::exit(0x01);
     }};
 }