From 3dd4e390bff5b7ed6adfba5c803d3c8381b57d72 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Mon, 30 Oct 2023 21:40:58 +0100 Subject: [PATCH] Refactor the hard timeout codepaths Makes it easier to monkey patch it for debug purposes. --- lib/pitchfork/children.rb | 27 +++++++++++++++++++++ lib/pitchfork/http_server.rb | 46 ++++++++++++++++-------------------- lib/pitchfork/worker.rb | 8 +++++++ 3 files changed, 55 insertions(+), 26 deletions(-) diff --git a/lib/pitchfork/children.rb b/lib/pitchfork/children.rb index 1a3debd4..a9b7687d 100644 --- a/lib/pitchfork/children.rb +++ b/lib/pitchfork/children.rb @@ -107,6 +107,33 @@ def each_worker(&block) @workers.each_value(&block) end + def soft_kill_all(sig) + each do |child| + child.soft_kill(sig) + end + end + + def hard_kill(sig, child) + child.hard_kill(sig) + rescue Errno::ESRCH + reap(child.pid) + child.close + end + + def hard_kill_all(sig) + each do |child| + hard_kill(sig, child) + end + end + + def hard_timeout(child) + child.hard_timeout! + rescue Errno::ESRCH + reap(child.pid) + child.close + true + end + def workers @workers.values end diff --git a/lib/pitchfork/http_server.rb b/lib/pitchfork/http_server.rb index 92adc32d..25e899c0 100644 --- a/lib/pitchfork/http_server.rb +++ b/lib/pitchfork/http_server.rb @@ -396,15 +396,15 @@ def stop(graceful = true) limit = Pitchfork.time_now + timeout until @children.workers.empty? || Pitchfork.time_now > limit if graceful - soft_kill_each_child(:TERM) + @children.soft_kill_all(:TERM) else - kill_each_child(:INT) + @children.hard_kill_all(:INT) end if monitor_loop(false) == StopIteration return StopIteration end end - kill_each_child(:KILL) + @children.hard_kill_all(:KILL) @promotion_lock.unlink end @@ -506,25 +506,28 @@ def murder_lazy_workers next else # worker is out of time next_sleep = 0 - if worker.mold? - logger.error "mold pid=#{worker.pid} timed out, killing" - else - logger.error "worker=#{worker.nr} pid=#{worker.pid} timed out, killing" - end + hard_timeout(worker) + end + end - if @after_worker_hard_timeout - begin - @after_worker_hard_timeout.call(self, worker) - rescue => error - Pitchfork.log_error(@logger, "after_worker_hard_timeout callback", error) - end - end + next_sleep <= 0 ? 1 : next_sleep + end - kill_worker(:KILL, worker.pid) # take no prisoners for hard timeout violations + def hard_timeout(worker) + if @after_worker_hard_timeout + begin + @after_worker_hard_timeout.call(self, worker) + rescue => error + Pitchfork.log_error(@logger, "after_worker_hard_timeout callback", error) end end - next_sleep <= 0 ? 1 : next_sleep + if worker.mold? + logger.error "mold pid=#{worker.pid} timed out, killing" + else + logger.error "worker=#{worker.nr} pid=#{worker.pid} timed out, killing" + end + @children.hard_timeout(worker) # take no prisoners for hard timeout violations end def trigger_refork @@ -930,15 +933,6 @@ def kill_worker(signal, wpid) worker = @children.reap(wpid) and worker.close rescue nil end - # delivers a signal to each worker - def kill_each_child(signal) - @children.each { |w| kill_worker(signal, w.pid) } - end - - def soft_kill_each_child(signal) - @children.each { |worker| worker.soft_kill(signal) } - end - # returns an array of string names for the given listener array def listener_names(listeners = LISTENERS) listeners.map { |io| sock_name(io) } diff --git a/lib/pitchfork/worker.rb b/lib/pitchfork/worker.rb index 0358e21e..810144f5 100644 --- a/lib/pitchfork/worker.rb +++ b/lib/pitchfork/worker.rb @@ -139,6 +139,14 @@ def soft_kill(sig) # :nodoc: success end + def hard_kill(sig) + Process.kill(sig, pid) + end + + def hard_timeout! + hard_kill(:KILL) + end + # this only runs when the Rack app.call is not running # act like a listener def accept_nonblock(exception: nil) # :nodoc: