From c9537dbebc8e7c63c4c229b66dae2ba6721b53db Mon Sep 17 00:00:00 2001 From: Shachar Langbeheim Date: Mon, 1 Apr 2024 14:49:23 +0300 Subject: [PATCH] Removed signals hook in socket. (#1201) The signals hook caused Node to not respond to ^C and similar signals, since Node yields to existing signals hooks. Tested Python, Node, C#, & Java benchmarks - all closed with ^C after this change. --- glide-core/Cargo.toml | 4 +-- glide-core/src/socket_listener.rs | 44 ++++++++----------------------- 2 files changed, 12 insertions(+), 36 deletions(-) diff --git a/glide-core/Cargo.toml b/glide-core/Cargo.toml index 9055b71d28..8e60ad0bb3 100644 --- a/glide-core/Cargo.toml +++ b/glide-core/Cargo.toml @@ -11,8 +11,6 @@ authors = ["Amazon Web Services"] bytes = { version = "^1.3", optional = true } futures = "^0.3" redis = { path = "../submodules/redis-rs/redis", features = ["aio", "tokio-comp", "tokio-rustls-comp", "connection-manager","cluster", "cluster-async"] } -signal-hook = { version = "^0.3", optional = true } -signal-hook-tokio = {version = "^0.3", features = ["futures-v0_3"], optional = true } tokio = { version = "1", features = ["macros", "time"] } logger_core = {path = "../logger_core"} dispose = "0.5.0" @@ -30,7 +28,7 @@ arcstr = "1.1.5" sha1_smol = "1.0.0" [features] -socket-layer = ["directories", "integer-encoding", "num_cpus", "signal-hook", "signal-hook-tokio", "protobuf", "tokio-util", "bytes", "rand"] +socket-layer = ["directories", "integer-encoding", "num_cpus", "protobuf", "tokio-util", "bytes", "rand"] [dev-dependencies] rsevents = "0.3.1" diff --git a/glide-core/src/socket_listener.rs b/glide-core/src/socket_listener.rs index 232ee3089c..10d1935200 100644 --- a/glide-core/src/socket_listener.rs +++ b/glide-core/src/socket_listener.rs @@ -14,7 +14,6 @@ use crate::response::Response; use crate::retry_strategies::get_fixed_interval_backoff; use directories::BaseDirs; use dispose::{Disposable, Dispose}; -use futures::stream::StreamExt; use logger_core::{log_debug, log_error, log_info, log_trace, log_warn}; use protobuf::Message; use redis::cluster_routing::{ @@ -23,8 +22,6 @@ use redis::cluster_routing::{ use redis::cluster_routing::{ResponsePolicy, Routable}; use redis::RedisError; use redis::{cmd, Cmd, Value}; -use signal_hook::consts::signal::*; -use signal_hook_tokio::Signals; use std::cell::Cell; use std::rc::Rc; use std::{env, str}; @@ -781,19 +778,17 @@ impl SocketListener { init_callback(Ok(self.socket_path.clone())); let local_set_pool = LocalPoolHandle::new(num_cpus::get()); loop { - tokio::select! { - listen_v = listener.accept() => { - if let Ok((stream, _addr)) = listen_v { - // New client - local_set_pool.spawn_pinned(move || { - listen_on_client_stream(stream) - }); - } else if listen_v.is_err() { - return - } - }, - // Interrupt was received, close the socket - _ = handle_signals() => return + match listener.accept().await { + Ok((stream, _addr)) => { + local_set_pool.spawn_pinned(move || listen_on_client_stream(stream)); + } + Err(err) => { + log_debug( + "listen_on_socket", + format!("Socket closed with error: `{err}`"), + ); + return; + } } } } @@ -879,23 +874,6 @@ pub fn get_socket_path() -> String { get_socket_path_from_name(socket_name) } -async fn handle_signals() { - // Handle Unix signals - let mut signals = - Signals::new([SIGTERM, SIGQUIT, SIGINT, SIGHUP]).expect("Failed creating signals"); - loop { - if let Some(signal) = signals.next().await { - match signal { - SIGTERM | SIGQUIT | SIGINT | SIGHUP => { - log_info("connection", format!("Signal {signal:?} received")); - return; - } - _ => continue, - } - } - } -} - /// This function is exposed only for the sake of testing with a nonstandard `socket_path`. /// Avoid using this function, unless you explicitly want to test the behavior of the listener /// without using the sockets used by other tests.