From dfc7e35b11ba8c59a702a5abfee5c79b8529d977 Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Fri, 8 Mar 2013 14:39:33 +0100 Subject: [PATCH] Refactor signal handling to use a pipe This commit removes the need to poll a global array for received signals every second by introducing an internal pipe(2). The writing end of the pipe is being used in the block trapping the signals. Whenever a signal is received, the signal's name gets written to the pipe. Instead of polling for received signals, the code now uses select(2) to check whether the internal pipe has been written to and can be read from. select(2) is blocking until one of the passed file descriptors is ready for reading/writing or has an exception, which means the code does not need to use `sleep 1` anymore. So when a signal is sent to the sidekiq process, the signal name gets written to the internal pipe, select(2) returns and the process can react according to the received signal. After handling a signal, select(2) gets called again and blocks until receiving another signal. --- lib/sidekiq/cli.rb | 74 ++++++++++++++++++++++------------------------ 1 file changed, 36 insertions(+), 38 deletions(-) diff --git a/lib/sidekiq/cli.rb b/lib/sidekiq/cli.rb index ffa18ba0..4cc516f4 100644 --- a/lib/sidekiq/cli.rb +++ b/lib/sidekiq/cli.rb @@ -1,11 +1,11 @@ -$sidekiq_signals = [] +$self_read, $self_write = IO.pipe # Signal handlers should do as little as humanly possible # and defer all work to a non-trap context. We'll have # the main thread poll for signals and handle them there. %w(INT TERM USR1 USR2 TTIN).each do |sig| trap sig do - $sidekiq_signals << sig + $self_write.puts(sig) end end @@ -69,9 +69,9 @@ module Sidekiq end launcher.run - while true - handle_signals - sleep 1 + while readable_io = IO.select([$self_read]) + signal = readable_io.first[0].gets.strip + handle_signal(signal) end rescue Interrupt logger.info 'Shutting down' @@ -82,40 +82,38 @@ module Sidekiq end end - def handle_signals - while sig = $sidekiq_signals.shift - Sidekiq.logger.debug "Got #{sig} signal" - case sig - when 'INT' - if Sidekiq.options[:profile] - result = RubyProf.stop - printer = RubyProf::GraphHtmlPrinter.new(result) - File.open("profile.html", 'w') do |f| - printer.print(f, :min_percent => 1) - end + def handle_signal(sig) + Sidekiq.logger.debug "Got #{sig} signal" + case sig + when 'INT' + if Sidekiq.options[:profile] + result = RubyProf.stop + printer = RubyProf::GraphHtmlPrinter.new(result) + File.open("profile.html", 'w') do |f| + printer.print(f, :min_percent => 1) end - # Handle Ctrl-C in JRuby like MRI - # http://jira.codehaus.org/browse/JRUBY-4637 - raise Interrupt - when 'TERM' - # Heroku sends TERM and then waits 10 seconds for process to exit. - raise Interrupt - when 'USR1' - Sidekiq.logger.info "Received USR1, no longer accepting new work" - launcher.manager.async.stop - when 'USR2' - if Sidekiq.options[:logfile] - Sidekiq.logger.info "Received USR2, reopening log file" - Sidekiq::Logging.initialize_logger(Sidekiq.options[:logfile]) - end - when 'TTIN' - Thread.list.each do |thread| - Sidekiq.logger.info "Thread TID-#{thread.object_id.to_s(36)} #{thread['label']}" - if thread.backtrace - Sidekiq.logger.info thread.backtrace.join("\n") - else - Sidekiq.logger.info "" - end + end + # Handle Ctrl-C in JRuby like MRI + # http://jira.codehaus.org/browse/JRUBY-4637 + raise Interrupt + when 'TERM' + # Heroku sends TERM and then waits 10 seconds for process to exit. + raise Interrupt + when 'USR1' + Sidekiq.logger.info "Received USR1, no longer accepting new work" + launcher.manager.async.stop + when 'USR2' + if Sidekiq.options[:logfile] + Sidekiq.logger.info "Received USR2, reopening log file" + Sidekiq::Logging.initialize_logger(Sidekiq.options[:logfile]) + end + when 'TTIN' + Thread.list.each do |thread| + Sidekiq.logger.info "Thread TID-#{thread.object_id.to_s(36)} #{thread['label']}" + if thread.backtrace + Sidekiq.logger.info thread.backtrace.join("\n") + else + Sidekiq.logger.info "" end end end