Refactored dispatch.fcgi. Added unit tests for dispatch.fcgi. Added trap to recognize HUP as a graceful termination command.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@1479 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

4 files changed, 202 insertions, 53 deletions

diff --git a/railties/dispatches/dispatch.fcgi b/railties/dispatches/dispatch.fcgi
index 34a99a7105..ed04839d6f 100755
--- a/railties/dispatches/dispatch.fcgi
+++ b/railties/dispatches/dispatch.fcgi
@@ -1,69 +1,99 @@
 #!/usr/local/bin/ruby
 
-def dispatcher_log(level, path,msg)
-  Logger.new(path).send(level, msg)
-rescue Object => log_error
-  STDERR << "Couldn't write to #{path}: #{msg}"
+# to allow unit testing
+if !defined?(RAILS_ROOT)
+  require File.dirname(__FILE__) + "/../config/environment"
 end
 
-def dispatcher_error(path,e,msg="")
-  error_message =
-    "[#{Time.now}] Dispatcher failed to catch: #{e} (#{e.class})\n  #{e.backtrace.join("\n  ")}\n#{msg}"
-  dispatcher_log(:error, path, error_message)
-end
+require 'dispatcher'
+require 'fcgi'
+require 'logger'
 
-last_error_on = nil
-begin
-  require File.dirname(__FILE__) + "/../config/environment"
-  require 'dispatcher'
-  require 'fcgi'
-
-  log_file_path = "#{RAILS_ROOT}/log/fastcgi.crash.log"
-  dispatcher_log(:info, log_file_path, "fcgi #{$$} starting")
-
-  # Allow graceful exits by sending the process SIGUSR1. If the process is
-  # currently handling a request, the request will be allowed to complete and
-  # then will terminate itself. If a request is not being handled, the
-  # process is terminated immediately (via #exit).
-
-  $please_exit_at_your_earliest_convenience = false
-  $i_am_currently_processing_a_request = false
-  trap("USR1") do
-    if $i_am_currently_processing_a_request
-      dispatcher_log(:info, log_file_path, "asking #{$$} to terminate ASAP")
-      $please_exit_at_your_earliest_convenience = true
+class RailsFCGIHandler
+  attr_reader :please_exit_at_your_earliest_convenience
+  attr_reader :i_am_currently_processing_a_request
+
+  def initialize(log_file_path = "#{RAILS_ROOT}/log/fastcgi.crash.log")
+    @please_exit_at_your_earliest_convenience = false
+    @i_am_currently_processing_a_request = false
+
+    trap_handler = method(:trap_handler).to_proc
+    trap("HUP",  trap_handler)
+    trap("USR1", trap_handler)
+
+    # initialize to 11 seconds from now to minimize special cases
+    @last_error_on = Time.now + 11
+
+    @log_file_path = log_file_path
+    dispatcher_log(:info, "fcgi #{$$} starting")
+  end
+
+  def process!
+    FCGI.each_cgi do |cgi| 
+      process_request(cgi)
+      break if please_exit_at_your_earliest_convenience
+    end
+
+    dispatcher_log(:info, "fcgi #{$$} terminated gracefully")
+
+  rescue SystemExit => exit_error
+    dispatcher_log(:info, "fcgi #{$$} terminated by explicit exit")
+
+  rescue Object => fcgi_error
+    # retry on errors that would otherwise have terminated the FCGI process,
+    # but only if they occur more than 10 seconds apart.
+    if !(SignalException === fcgi_error) && @last_error_on - Time.now > 10
+      @last_error_on = Time.now
+      dispatcher_error(fcgi_error,
+        "FCGI process #{$$} almost killed by this error\n")
+      retry
     else
-      dispatcher_log(:info, log_file_path, "telling #{$$} to terminate NOW")
-      exit
+      dispatcher_error(fcgi_error, "FCGI process #{$$} killed by this error\n")
     end
   end
 
-  # Process each request as it comes in, as a pseudo-CGI.
+  private
+    def logger
+      @logger ||= Logger.new(@log_file_path)
+    end
+
+    def dispatcher_log(level, msg)
+      logger.send(level, msg)
+    rescue Object => log_error
+      STDERR << "Couldn't write to #{@log_file_path.inspect}: #{msg}\n"
+      STDERR << "  #{log_error.class}: #{log_error.message}\n"
+    end
+
+    def dispatcher_error(e,msg="")
+      error_message =
+        "[#{Time.now}] Dispatcher failed to catch: #{e} (#{e.class})\n" +
+        "  #{e.backtrace.join("\n  ")}\n#{msg}"
+      dispatcher_log(:error, error_message)
+    end
+
+    def trap_handler
+      if i_am_currently_processing_a_request
+        dispatcher_log(:info, "asking #{$$} to terminate ASAP")
+        @please_exit_at_your_earliest_convenience = true
+      else
+        dispatcher_log(:info, "telling #{$$} to terminate NOW")
+        exit
+      end
+    end
 
-  FCGI.each_cgi do |cgi| 
-    begin
-      $i_am_currently_processing_a_request = true
+    def process_request(cgi)
+      @i_am_currently_processing_a_request = true
       Dispatcher.dispatch(cgi)
     rescue Object => e
-      dispatcher_error(log_file_path, e)
+      raise if SignalException === e
+      dispatcher_error(e)
     ensure
       $stdout.flush
-      $i_am_currently_processing_a_request = false
-      break if $please_exit_at_your_earliest_convenience
+      @i_am_currently_processing_a_request = false
     end
-  end
+end
 
-  dispatcher_log(:info, log_file_path, "fcgi #{$$} terminated gracefully")
-rescue SystemExit => exit_error
-  dispatcher_log(:info, log_file_path, "fcgi #{$$} terminated by explicit exit")
-rescue Object => fcgi_error
-  # retry on errors that would otherwise have terminated the FCGI process, but
-  # only if they occur more than 10 seconds apart.
-  if !(SignalException === fcgi_error) && (last_error_on.nil? || last_error_on - Time.now > 10)
-    last_error_on = Time.now
-    dispatcher_error(log_file_path, fcgi_error, "FCGI process #{$$} almost killed by this error\n")
-    retry
-  else
-    dispatcher_error(log_file_path, fcgi_error, "FCGI process #{$$} killed by this error\n")
-  end
-end
\ No newline at end of file
+if __FILE__ == $0
+  handler = RailsFCGIHandler.new
+  handler.process!
+end
diff --git a/railties/test/fcgi_dispatcher_test.rb b/railties/test/fcgi_dispatcher_test.rb
new file mode 100644
index 0000000000..3aeb447fa1
--- /dev/null
+++ b/railties/test/fcgi_dispatcher_test.rb
@@ -0,0 +1,96 @@
+$:.unshift File.dirname(__FILE__) + "/mocks"
+
+require 'test/unit'
+require 'stringio'
+
+if !defined?(RailsFCGIHandler)
+  RAILS_ROOT = File.dirname(__FILE__)
+  load File.dirname(__FILE__) + "/../dispatches/dispatch.fcgi"
+end
+
+class RailsFCGIHandler
+  attr_reader :exit_code
+  attr_accessor :thread
+
+  def trap(signal, handler, &block)
+    handler ||= block
+    (@signal_handlers ||= Hash.new)[signal] = handler
+  end
+
+  def exit(code=0)
+    @exit_code = code
+    (thread || Thread.current).exit
+  end
+
+  def send_signal(which)
+    @signal_handlers[which].call
+  end
+end
+
+class RailsFCGIHandlerTest < Test::Unit::TestCase
+  def setup
+    @log = StringIO.new
+    @handler = RailsFCGIHandler.new(@log)
+    FCGI.time_to_sleep = nil
+    FCGI.raise_exception = nil
+    Dispatcher.time_to_sleep = nil
+    Dispatcher.raise_exception = nil
+  end
+
+  def test_uninterrupted_processing
+    @handler.process!
+    assert_nil @handler.exit_code
+    assert !@handler.please_exit_at_your_earliest_convenience
+    assert !@handler.i_am_currently_processing_a_request
+  end
+
+  %w(HUP USR1).each do |signal|
+    define_method("test_interrupted_via_#{signal}_when_not_in_request") do
+      FCGI.time_to_sleep = 1
+      @handler.thread = Thread.new { @handler.process! }
+      sleep 0.1 # let the thread get started
+      @handler.send_signal(signal)
+      @handler.thread.join
+      assert_equal 0, @handler.exit_code
+      assert !@handler.please_exit_at_your_earliest_convenience
+      assert !@handler.i_am_currently_processing_a_request
+    end
+
+    define_method("test_interrupted_via_#{signal}_when_in_request") do
+      Dispatcher.time_to_sleep = 1
+      @handler.thread = Thread.new { @handler.process! }
+      sleep 0.1 # let the thread get started
+      @handler.send_signal(signal)
+      @handler.thread.join
+      assert_nil @handler.exit_code
+      assert @handler.please_exit_at_your_earliest_convenience
+      assert !@handler.i_am_currently_processing_a_request
+    end
+  end
+
+  %w(RuntimeError SignalException).each do |exception|
+    define_method("test_#{exception}_in_fcgi") do
+      FCGI.raise_exception = Object.const_get(exception)
+      @handler.process!
+      assert_match %r{Dispatcher failed to catch}, @log.string
+      case exception
+        when "RuntimeError"
+          assert_match %r{almost killed}, @log.string
+        when "SignalException"
+          assert_match %r{\d killed}, @log.string
+      end
+    end
+
+    define_method("test_#{exception}_in_dispatcher") do
+      Dispatcher.raise_exception = Object.const_get(exception)
+      @handler.process!
+      assert_match %r{Dispatcher failed to catch}, @log.string
+      case exception
+        when "RuntimeError"
+          assert_no_match %r{killed}, @log.string
+        when "SignalException"
+          assert_match %r{\d killed}, @log.string
+      end
+    end
+  end
+end
diff --git a/railties/test/mocks/dispatcher.rb b/railties/test/mocks/dispatcher.rb
new file mode 100644
index 0000000000..9ca6b609c6
--- /dev/null
+++ b/railties/test/mocks/dispatcher.rb
@@ -0,0 +1,11 @@
+class Dispatcher
+  class <<self
+    attr_accessor :time_to_sleep
+    attr_accessor :raise_exception
+
+    def dispatch(cgi)
+      sleep(time_to_sleep || 0)
+      raise raise_exception, "Something died" if raise_exception
+    end
+  end
+end
diff --git a/railties/test/mocks/fcgi.rb b/railties/test/mocks/fcgi.rb
new file mode 100644
index 0000000000..071b8e1848
--- /dev/null
+++ b/railties/test/mocks/fcgi.rb
@@ -0,0 +1,12 @@
+class FCGI
+  class << self
+    attr_accessor :time_to_sleep
+    attr_accessor :raise_exception
+
+    def each_cgi
+      sleep(time_to_sleep || 0)
+      raise raise_exception, "Something died" if raise_exception
+      yield "mock cgi value"
+    end
+  end
+end