From 66ba48091e857603fb9dd9ad1a670f301c7ac8cd Mon Sep 17 00:00:00 2001 From: Vyacheslav Alexeev Date: Tue, 14 Dec 2021 13:27:00 +0800 Subject: [PATCH 1/9] implement safe signal trap && use it --- lib/puma/cluster.rb | 12 ++++++------ lib/puma/cluster/worker.rb | 8 ++++---- lib/puma/launcher.rb | 13 +++++++------ lib/puma/util.rb | 16 ++++++++++++++++ 4 files changed, 33 insertions(+), 16 deletions(-) diff --git a/lib/puma/cluster.rb b/lib/puma/cluster.rb index 7d6d12189e..44062fea92 100644 --- a/lib/puma/cluster.rb +++ b/lib/puma/cluster.rb @@ -266,7 +266,7 @@ def fork_worker! # of the signals handlers as small as possible. def setup_signals if @options[:fork_worker] - Signal.trap "SIGURG" do + Puma::Util.safe_signal_trap "SIGURG" do fork_worker! end @@ -280,23 +280,23 @@ def setup_signals end end - Signal.trap "SIGCHLD" do + Puma::Util.safe_signal_trap "SIGCHLD" do wakeup! end - Signal.trap "TTIN" do + Puma::Util.safe_signal_trap "TTIN" do @options[:workers] += 1 wakeup! end - Signal.trap "TTOU" do + Puma::Util.safe_signal_trap "TTOU" do @options[:workers] -= 1 if @options[:workers] >= 2 wakeup! end master_pid = Process.pid - Signal.trap "SIGTERM" do + Puma::Util.safe_signal_trap "SIGTERM" do # The worker installs their own SIGTERM when booted. # Until then, this is run by the worker and the worker # should just exit if they get it. @@ -396,7 +396,7 @@ def run spawn_workers - Signal.trap "SIGINT" do + Puma::Util.safe_signal_trap "SIGINT" do stop end diff --git a/lib/puma/cluster/worker.rb b/lib/puma/cluster/worker.rb index 7ac46883db..3200f4ed22 100644 --- a/lib/puma/cluster/worker.rb +++ b/lib/puma/cluster/worker.rb @@ -30,8 +30,8 @@ def run title += " [#{@options[:tag]}]" if @options[:tag] && !@options[:tag].empty? $0 = title - Signal.trap "SIGINT", "IGNORE" - Signal.trap "SIGCHLD", "DEFAULT" + Puma::Util.safe_signal_trap "SIGINT", "IGNORE" + Puma::Util.safe_signal_trap.trap "SIGCHLD", "DEFAULT" Thread.new do Puma.set_thread_name "wrkr check" @@ -69,7 +69,7 @@ def run if fork_worker restart_server.clear worker_pids = [] - Signal.trap "SIGCHLD" do + Puma::Util.safe_signal_trap "SIGCHLD" do wakeup! if worker_pids.reject! do |p| Process.wait(p, Process::WNOHANG) rescue true end @@ -96,7 +96,7 @@ def run end end - Signal.trap "SIGTERM" do + Puma::Util.safe_signal_trap "SIGTERM" do @worker_write << "e#{Process.pid}\n" rescue nil restart_server.clear server.stop diff --git a/lib/puma/launcher.rb b/lib/puma/launcher.rb index 589a580a45..fcbb0e2d0e 100644 --- a/lib/puma/launcher.rb +++ b/lib/puma/launcher.rb @@ -6,6 +6,7 @@ require 'puma/single' require 'puma/const' require 'puma/binder' +require 'puma/util' module Puma # Puma::Launcher is the single entry point for starting a Puma server based on user @@ -454,7 +455,7 @@ def generate_restart_data def setup_signals begin - Signal.trap "SIGUSR2" do + Puma::Util.safe_signal_trap "SIGUSR2" do restart end rescue Exception @@ -463,7 +464,7 @@ def setup_signals unless Puma.jruby? begin - Signal.trap "SIGUSR1" do + Puma::Util.safe_signal_trap "SIGUSR1" do phased_restart end rescue Exception @@ -472,7 +473,7 @@ def setup_signals end begin - Signal.trap "SIGTERM" do + Puma::Util.safe_signal_trap "SIGTERM" do graceful_stop raise(SignalException, "SIGTERM") if @options[:raise_exception_on_sigterm] @@ -482,7 +483,7 @@ def setup_signals end begin - Signal.trap "SIGINT" do + Puma::Util.safe_signal_trap "SIGINT" do stop end rescue Exception @@ -490,7 +491,7 @@ def setup_signals end begin - Signal.trap "SIGHUP" do + Puma::Util.safe_signal_trap "SIGHUP" do if @runner.redirected_io? @runner.redirect_io else @@ -503,7 +504,7 @@ def setup_signals begin unless Puma.jruby? # INFO in use by JVM already - Signal.trap "SIGINFO" do + Puma::Util.safe_signal_trap "SIGINFO" do thread_status do |name, backtrace| @events.log name @events.log backtrace.map { |bt| " #{bt}" } diff --git a/lib/puma/util.rb b/lib/puma/util.rb index bf1cabc176..ff1b3840ad 100644 --- a/lib/puma/util.rb +++ b/lib/puma/util.rb @@ -71,6 +71,22 @@ def parse_query(qs, d = nil, &unescaper) params end + # Signal.trap that does not replace + # the existing puma handlers + def safe_signal_trap(sig) + old_handler = Signal.trap(sig) do + if old_handler.respond_to?(:call) + begin + old_handler.call + rescue Exception + puts $!.inspect + end + end + + yield + end + end + # A case-insensitive Hash that preserves the original case of a # header when set. class HeaderHash < Hash From 73d8e3251ca6db718d2a356adce167d40c37447c Mon Sep 17 00:00:00 2001 From: Vyacheslav Alexeev Date: Tue, 14 Dec 2021 14:00:08 +0800 Subject: [PATCH 2/9] do not log on rescue --- lib/puma/util.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/puma/util.rb b/lib/puma/util.rb index ff1b3840ad..8d0a06bed2 100644 --- a/lib/puma/util.rb +++ b/lib/puma/util.rb @@ -79,7 +79,6 @@ def safe_signal_trap(sig) begin old_handler.call rescue Exception - puts $!.inspect end end From f4fda9fcc12a6073a8fc5cfb365f4235738bf1c0 Mon Sep 17 00:00:00 2001 From: Vyacheslav Alexeev Date: Tue, 14 Dec 2021 14:12:02 +0800 Subject: [PATCH 3/9] hotfix --- lib/puma/cluster/worker.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/puma/cluster/worker.rb b/lib/puma/cluster/worker.rb index 3200f4ed22..0d35a5a7be 100644 --- a/lib/puma/cluster/worker.rb +++ b/lib/puma/cluster/worker.rb @@ -31,7 +31,7 @@ def run $0 = title Puma::Util.safe_signal_trap "SIGINT", "IGNORE" - Puma::Util.safe_signal_trap.trap "SIGCHLD", "DEFAULT" + Puma::Util.safe_signal_trap "SIGCHLD", "DEFAULT" Thread.new do Puma.set_thread_name "wrkr check" From 9d157fbc431a96ee239abe85309fc6227c8fd7f7 Mon Sep 17 00:00:00 2001 From: Vyacheslav Alexeev Date: Tue, 14 Dec 2021 14:58:33 +0800 Subject: [PATCH 4/9] hotfix --- lib/puma/cluster/worker.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/puma/cluster/worker.rb b/lib/puma/cluster/worker.rb index 0d35a5a7be..295da58e31 100644 --- a/lib/puma/cluster/worker.rb +++ b/lib/puma/cluster/worker.rb @@ -30,10 +30,10 @@ def run title += " [#{@options[:tag]}]" if @options[:tag] && !@options[:tag].empty? $0 = title - Puma::Util.safe_signal_trap "SIGINT", "IGNORE" - Puma::Util.safe_signal_trap "SIGCHLD", "DEFAULT" + Signal.trap "SIGINT", "IGNORE" + Signal.trap "SIGCHLD", "DEFAULT" - Thread.new do + Thread.new do Puma.set_thread_name "wrkr check" @check_pipe.wait_readable log "! Detected parent died, dying" From 911cb109b29ed07915023d2d57dcf3bf5cd00335 Mon Sep 17 00:00:00 2001 From: Vyacheslav Alexeev Date: Fri, 17 Dec 2021 00:33:50 +0800 Subject: [PATCH 5/9] new Puma::Signal solution --- lib/puma/cluster.rb | 13 ++++++------ lib/puma/cluster/worker.rb | 14 ++++++++----- lib/puma/launcher.rb | 13 ++++++------ lib/puma/signal.rb | 42 ++++++++++++++++++++++++++++++++++++++ lib/puma/util.rb | 15 -------------- 5 files changed, 65 insertions(+), 32 deletions(-) create mode 100644 lib/puma/signal.rb diff --git a/lib/puma/cluster.rb b/lib/puma/cluster.rb index 44062fea92..6f42c2393c 100644 --- a/lib/puma/cluster.rb +++ b/lib/puma/cluster.rb @@ -5,6 +5,7 @@ require 'puma/plugin' require 'puma/cluster/worker_handle' require 'puma/cluster/worker' +require 'puma/signal' require 'time' @@ -266,7 +267,7 @@ def fork_worker! # of the signals handlers as small as possible. def setup_signals if @options[:fork_worker] - Puma::Util.safe_signal_trap "SIGURG" do + Puma::Signal.trap "SIGURG" do fork_worker! end @@ -280,23 +281,23 @@ def setup_signals end end - Puma::Util.safe_signal_trap "SIGCHLD" do + Puma::Signal.trap "SIGCHLD" do wakeup! end - Puma::Util.safe_signal_trap "TTIN" do + Puma::Signal.trap "TTIN" do @options[:workers] += 1 wakeup! end - Puma::Util.safe_signal_trap "TTOU" do + Puma::Signal.trap "TTOU" do @options[:workers] -= 1 if @options[:workers] >= 2 wakeup! end master_pid = Process.pid - Puma::Util.safe_signal_trap "SIGTERM" do + Puma::Signal.trap "SIGTERM" do # The worker installs their own SIGTERM when booted. # Until then, this is run by the worker and the worker # should just exit if they get it. @@ -396,7 +397,7 @@ def run spawn_workers - Puma::Util.safe_signal_trap "SIGINT" do + Puma::Signal.trap "SIGINT" do stop end diff --git a/lib/puma/cluster/worker.rb b/lib/puma/cluster/worker.rb index 295da58e31..6f84faaa0a 100644 --- a/lib/puma/cluster/worker.rb +++ b/lib/puma/cluster/worker.rb @@ -30,8 +30,12 @@ def run title += " [#{@options[:tag]}]" if @options[:tag] && !@options[:tag].empty? $0 = title - Signal.trap "SIGINT", "IGNORE" - Signal.trap "SIGCHLD", "DEFAULT" + Puma::Signal.trap "SIGINT" do + end + + Puma::Signal.trap "SIGCHLD" do |signal| + raise SignalException, signal + end Thread.new do Puma.set_thread_name "wrkr check" @@ -55,7 +59,7 @@ def run @launcher.config.run_hooks :before_worker_boot, index, @launcher.events begin - server = @server ||= start_server + server = @server ||= start_server rescue Exception => e log "! Unable to start worker" log e.backtrace[0] @@ -69,7 +73,7 @@ def run if fork_worker restart_server.clear worker_pids = [] - Puma::Util.safe_signal_trap "SIGCHLD" do + Puma::Signal.trap "SIGCHLD" do wakeup! if worker_pids.reject! do |p| Process.wait(p, Process::WNOHANG) rescue true end @@ -96,7 +100,7 @@ def run end end - Puma::Util.safe_signal_trap "SIGTERM" do + Puma::Signal.trap "SIGTERM" do @worker_write << "e#{Process.pid}\n" rescue nil restart_server.clear server.stop diff --git a/lib/puma/launcher.rb b/lib/puma/launcher.rb index fcbb0e2d0e..04f564c7c5 100644 --- a/lib/puma/launcher.rb +++ b/lib/puma/launcher.rb @@ -7,6 +7,7 @@ require 'puma/const' require 'puma/binder' require 'puma/util' +require 'puma/signal' module Puma # Puma::Launcher is the single entry point for starting a Puma server based on user @@ -455,7 +456,7 @@ def generate_restart_data def setup_signals begin - Puma::Util.safe_signal_trap "SIGUSR2" do + Puma::Signal.trap "SIGUSR2" do restart end rescue Exception @@ -464,7 +465,7 @@ def setup_signals unless Puma.jruby? begin - Puma::Util.safe_signal_trap "SIGUSR1" do + Puma::Signal.trap "SIGUSR1" do phased_restart end rescue Exception @@ -473,7 +474,7 @@ def setup_signals end begin - Puma::Util.safe_signal_trap "SIGTERM" do + Puma::Signal.trap "SIGTERM" do graceful_stop raise(SignalException, "SIGTERM") if @options[:raise_exception_on_sigterm] @@ -483,7 +484,7 @@ def setup_signals end begin - Puma::Util.safe_signal_trap "SIGINT" do + Puma::Signal.trap "SIGINT" do stop end rescue Exception @@ -491,7 +492,7 @@ def setup_signals end begin - Puma::Util.safe_signal_trap "SIGHUP" do + Puma::Signal.trap "SIGHUP" do if @runner.redirected_io? @runner.redirect_io else @@ -504,7 +505,7 @@ def setup_signals begin unless Puma.jruby? # INFO in use by JVM already - Puma::Util.safe_signal_trap "SIGINFO" do + Puma::Signal.trap "SIGINFO" do thread_status do |name, backtrace| @events.log name @events.log backtrace.map { |bt| " #{bt}" } diff --git a/lib/puma/signal.rb b/lib/puma/signal.rb new file mode 100644 index 0000000000..925678d7bf --- /dev/null +++ b/lib/puma/signal.rb @@ -0,0 +1,42 @@ +module Puma + module Signal + module_function + + def prepend_handler(sig, &handler) + name = signame(sig) + custom_signal_handlers[signame] ||= [] + custom_signal_handlers[signame].prepend(handler) + end + + # Signal.trap that does not replace + # the existing puma handlers + def trap(sig, &handler) + name = signame(sig) + ::Signal.trap(name) do + invoke_custom_signal_handlers(name) + handler.call + end + end + + private + + def signame(sig) + name = sig.is_a?(Integer) ? ::Signal.signame(sig) : String(sig).sub('SIG', '') + raise ArgumentError, "unsupported signal SIG#{name}" unless ::Signal.list[name] + name + end + module_function :signame + + def invoke_custom_signal_handlers(signame) + Array(custom_signal_handlers[signame]).each do |handler| + handler.call + end + end + module_function :invoke_custom_signal_handlers + + def custom_signal_handlers + @custom_signal_handlers ||= {} + end + module_function :custom_signal_handlers + end +end diff --git a/lib/puma/util.rb b/lib/puma/util.rb index 8d0a06bed2..bf1cabc176 100644 --- a/lib/puma/util.rb +++ b/lib/puma/util.rb @@ -71,21 +71,6 @@ def parse_query(qs, d = nil, &unescaper) params end - # Signal.trap that does not replace - # the existing puma handlers - def safe_signal_trap(sig) - old_handler = Signal.trap(sig) do - if old_handler.respond_to?(:call) - begin - old_handler.call - rescue Exception - end - end - - yield - end - end - # A case-insensitive Hash that preserves the original case of a # header when set. class HeaderHash < Hash From 06b0d5bcec2b219f94ed039e525ab8f5b0b9ae9f Mon Sep 17 00:00:00 2001 From: Vyacheslav Alexeev Date: Fri, 17 Dec 2021 00:34:53 +0800 Subject: [PATCH 6/9] clean --- lib/puma/launcher.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/puma/launcher.rb b/lib/puma/launcher.rb index 04f564c7c5..f344cb951b 100644 --- a/lib/puma/launcher.rb +++ b/lib/puma/launcher.rb @@ -6,7 +6,6 @@ require 'puma/single' require 'puma/const' require 'puma/binder' -require 'puma/util' require 'puma/signal' module Puma From 2f3562e31b9d3df6720d025e7cfab5b9af936575 Mon Sep 17 00:00:00 2001 From: Vyacheslav Alexeev Date: Fri, 17 Dec 2021 00:44:48 +0800 Subject: [PATCH 7/9] linter --- lib/puma/signal.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/puma/signal.rb b/lib/puma/signal.rb index 925678d7bf..369e223f12 100644 --- a/lib/puma/signal.rb +++ b/lib/puma/signal.rb @@ -14,7 +14,7 @@ def trap(sig, &handler) name = signame(sig) ::Signal.trap(name) do invoke_custom_signal_handlers(name) - handler.call + yield handler end end @@ -29,7 +29,7 @@ def signame(sig) def invoke_custom_signal_handlers(signame) Array(custom_signal_handlers[signame]).each do |handler| - handler.call + yield handler end end module_function :invoke_custom_signal_handlers From 7b45e05ca1df2e23ab22051e2629b1066fa166cb Mon Sep 17 00:00:00 2001 From: Vyacheslav Alexeev Date: Sun, 1 May 2022 21:13:44 +0200 Subject: [PATCH 8/9] add behavior for text signal handlers --- lib/puma/cluster/worker.rb | 7 ++----- lib/puma/signal.rb | 13 +++++++++++-- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/lib/puma/cluster/worker.rb b/lib/puma/cluster/worker.rb index 934625aee8..f921dda610 100644 --- a/lib/puma/cluster/worker.rb +++ b/lib/puma/cluster/worker.rb @@ -30,12 +30,9 @@ def run title += " [#{@options[:tag]}]" if @options[:tag] && !@options[:tag].empty? $0 = title - Puma::Signal.trap "SIGINT" do - end + Puma::Signal.trap "SIGINT", "IGNORE" - Puma::Signal.trap "SIGCHLD" do |signal| - raise SignalException, signal - end + Puma::Signal.trap "SIGCHLD", "DEFAULT" Thread.new do Puma.set_thread_name "wrkr check" diff --git a/lib/puma/signal.rb b/lib/puma/signal.rb index 369e223f12..b89c47b413 100644 --- a/lib/puma/signal.rb +++ b/lib/puma/signal.rb @@ -10,11 +10,20 @@ def prepend_handler(sig, &handler) # Signal.trap that does not replace # the existing puma handlers - def trap(sig, &handler) + def trap(sig, handler_str=nil, &handler) name = signame(sig) ::Signal.trap(name) do invoke_custom_signal_handlers(name) - yield handler + + if handler.respond_to?(:call) + handler.call + else + # dirty hack to call string handlers + # especially for DEFAULT and SYSTEM_DEFAULT + current_trap = ::Signal.trap(name, handler_str) + Process.kill(name, Process.pid) + ::Signal.trap(name, current_trap) + end end end From 2e618669f2094ea13d9e37b3be89f66b14030c95 Mon Sep 17 00:00:00 2001 From: Vyacheslav Alexeev Date: Sun, 1 May 2022 21:30:56 +0200 Subject: [PATCH 9/9] hotfix --- lib/puma/signal.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/puma/signal.rb b/lib/puma/signal.rb index b89c47b413..4b78841b36 100644 --- a/lib/puma/signal.rb +++ b/lib/puma/signal.rb @@ -16,7 +16,7 @@ def trap(sig, handler_str=nil, &handler) invoke_custom_signal_handlers(name) if handler.respond_to?(:call) - handler.call + yield handler else # dirty hack to call string handlers # especially for DEFAULT and SYSTEM_DEFAULT