r1588@asus: jeremy | 2005-07-02 03:14:45 -0700

 Optional periodic garbage collection for dispatch.fcgi.  Graceful exit on TERM also (a la Apache1).  Ignore signals the platform does not support, such as USR1 on Windows.


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

6 files changed, 161 insertions, 21 deletions

diff --git a/railties/CHANGELOG b/railties/CHANGELOG
index 437f78bba8..18ab2ba0b9 100644
--- a/railties/CHANGELOG
+++ b/railties/CHANGELOG
@@ -1,5 +1,9 @@
 *SVN*
 
+* SIGTERM also gracefully exits dispatch.fcgi.  Ignore SIGUSR1 on Windows.
+
+* Add the option to manually manage garbage collection in the FastCGI dispatcher.  Set the number of requests between GC runs in your public/dispatch.fcgi.  [skaes@web.de]
+
 * Allow dynamic application reloading for dispatch.fcgi processes by sending a SIGHUP. If the process is currently handling a request, the request will be allowed to complete first. This allows production fcgi's to be reloaded without having to restart them.
 
 * RailsFCGIHandler (dispatch.fcgi) no longer tries to explicitly flush $stdout (CgiProcess#out always calls flush)
diff --git a/railties/dispatches/dispatch.fcgi b/railties/dispatches/dispatch.fcgi
index 5164669f2e..65188f380b 100755
--- a/railties/dispatches/dispatch.fcgi
+++ b/railties/dispatches/dispatch.fcgi
@@ -1,5 +1,23 @@
 #!/usr/local/bin/ruby
-
+#
+# You may specify the path to the FastCGI crash log (a log of unhandled
+# exceptions which forced the FastCGI instance to exit, great for debugging)
+# and the number of requests to process before running garbage collection.
+#
+# By default, the FastCGI crash log is RAILS_ROOT/log/fastcgi.crash.log
+# and the GC period is nil (turned off).  A reasonable number of requests
+# could range from 10-100 depending on the memory footprint of your app.
+#
+# Example:
+#   # Default log path, normal GC behavior.
+#   RailsFCGIHandler.process!
+#
+#   # Default log path, 50 requests between GC.
+#   RailsFCGIHandler.process! nil, 50
+#
+#   # Custom log path, normal GC behavior.
+#   RailsFCGIHandler.process! '/var/log/myapp_fcgi_crash.log'
+#
 require File.dirname(__FILE__) + "/../config/environment"
 require 'fcgi_handler'
 
diff --git a/railties/lib/fcgi_handler.rb b/railties/lib/fcgi_handler.rb
index f615ff6d34..f489627985 100644
--- a/railties/lib/fcgi_handler.rb
+++ b/railties/lib/fcgi_handler.rb
@@ -3,41 +3,77 @@ require 'logger'
 require 'dispatcher'
 
 class RailsFCGIHandler
+  SIGNALS = {
+    'HUP'  => :reload,
+    'TERM' => :graceful_exit,
+    'USR1' => :graceful_exit
+  }
+
   attr_reader :when_ready
   attr_reader :processing
 
-  def self.process!
-    new.process!
+  attr_accessor :log_file_path
+  attr_accessor :gc_request_period
+
+
+  # Initialize and run the FastCGI instance, passing arguments through to new.
+  def self.process!(*args, &block)
+    new(*args, &block).process!
   end
 
-  def initialize(log_file_path = "#{RAILS_ROOT}/log/fastcgi.crash.log")
+  # Initialize the FastCGI instance with the path to a crash log
+  # detailing unhandled exceptions (default RAILS_ROOT/log/fastcgi.crash.log)
+  # and the number of requests to process between garbage collection runs
+  # (default nil for normal GC behavior.)  Optionally, pass a block which
+  # takes this instance as an argument for further configuration.
+  def initialize(log_file_path = nil, gc_request_period = nil)
     @when_ready = nil
     @processing = false
 
-    trap("HUP",  method(:restart_handler).to_proc)
-    trap("USR1", method(:trap_handler).to_proc)
+    self.log_file_path = log_file_path || "#{RAILS_ROOT}/log/fastcgi.crash.log"
+    self.gc_request_period = gc_request_period
+
+    # Yield for additional configuration.
+    yield self if block_given?
 
-    # initialize to 11 seconds ago to minimize special cases
+    # Safely install signal handlers.
+    install_signal_handlers
+
+    # Start error timestamp at 11 seconds ago.
     @last_error_on = Time.now - 11
 
-    @log_file_path = log_file_path
     dispatcher_log(:info, "starting")
   end
 
   def process!
+    # Make a note of $" so we can safely reload this instance.
     mark!
 
+    # Begin countdown to garbage collection.
+    run_gc! if gc_request_period
+
     FCGI.each_cgi do |cgi| 
-      if when_ready == :restart
+      # Safely reload this instance if requested.
+      if when_ready == :reload
+        run_gc! if gc_request_period
         restore!
         @when_ready = nil
-        dispatcher_log(:info, "restarted")
+        dispatcher_log(:info, "reloaded")
       end
 
       process_request(cgi)
+
+      # Break if graceful exit requested.
       break if when_ready == :exit
+
+      # Garbage collection countdown.
+      if gc_request_period
+        @gc_request_countdown -= 1
+        run_gc! if @gc_request_countdown <= 0
+      end
     end
 
+    GC.enable
     dispatcher_log(:info, "terminated gracefully")
 
   rescue SystemExit => exit_error
@@ -75,7 +111,19 @@ class RailsFCGIHandler
       dispatcher_log(:error, error_message)
     end
 
-    def trap_handler(signal)
+    def install_signal_handlers
+      SIGNALS.each do |signal, handler_name|
+        install_signal_handler signal, method("#{handler_name}_handler").to_proc
+      end
+    end
+
+    def install_signal_handler(signal, handler)
+      trap signal, handler
+    rescue ArgumentError
+      dispatcher_log :warn, "Ignoring unsupported signal #{signal}."
+    end
+
+    def graceful_exit_handler(signal)
       if processing
         dispatcher_log :info, "asked to terminate ASAP"
         @when_ready = :exit
@@ -85,9 +133,9 @@ class RailsFCGIHandler
       end
     end
 
-    def restart_handler(signal)
-      @when_ready = :restart
-      dispatcher_log :info, "asked to restart ASAP"
+    def reload_handler(signal)
+      @when_ready = :reload
+      dispatcher_log :info, "asked to reload ASAP"
     end
 
     def process_request(cgi)
@@ -109,4 +157,9 @@ class RailsFCGIHandler
       Dispatcher.reset_application!
       ActionController::Routing::Routes.reload
     end
+
+    def run_gc!
+      @gc_request_countdown = gc_request_period
+      GC.enable; GC.start; GC.disable
+    end
 end
diff --git a/railties/test/fcgi_dispatcher_test.rb b/railties/test/fcgi_dispatcher_test.rb
index d9f147b13d..1d9b6fafaf 100644
--- a/railties/test/fcgi_dispatcher_test.rb
+++ b/railties/test/fcgi_dispatcher_test.rb
@@ -9,8 +9,9 @@ RAILS_ROOT = File.dirname(__FILE__) if !defined?(RAILS_ROOT)
 
 class RailsFCGIHandler
   attr_reader :exit_code
-  attr_reader :restarted
+  attr_reader :reloaded
   attr_accessor :thread
+  attr_reader :gc_runs
 
   def trap(signal, handler, &block)
     handler ||= block
@@ -27,7 +28,14 @@ class RailsFCGIHandler
   end
 
   def restore!
-    @restarted = true
+    @reloaded = true
+  end
+
+  alias_method :old_run_gc!, :run_gc!
+  def run_gc!
+    @gc_runs ||= 0
+    @gc_runs += 1
+    old_run_gc!
   end
 end
 
@@ -57,7 +65,7 @@ class RailsFCGIHandlerTest < Test::Unit::TestCase
     assert_nil @handler.exit_code
     assert_nil @handler.when_ready
     assert !@handler.processing
-    assert @handler.restarted
+    assert @handler.reloaded
   end
 
   def test_interrupted_via_HUP_when_in_request
@@ -67,7 +75,7 @@ class RailsFCGIHandlerTest < Test::Unit::TestCase
     @handler.send_signal("HUP")
     @handler.thread.join
     assert_nil @handler.exit_code
-    assert_equal :restart, @handler.when_ready
+    assert_equal :reload, @handler.when_ready
     assert !@handler.processing
   end
 
@@ -119,3 +127,55 @@ class RailsFCGIHandlerTest < Test::Unit::TestCase
     end
   end
 end
+
+class RailsFCGIHandlerPeriodicGCTest < Test::Unit::TestCase
+  def setup
+    @log = StringIO.new
+    FCGI.time_to_sleep = nil
+    FCGI.raise_exception = nil
+    FCGI.each_cgi_count = nil
+    Dispatcher.time_to_sleep = nil
+    Dispatcher.raise_exception = nil
+    Dispatcher.dispatch_hook = nil
+  end
+
+  def teardown
+    FCGI.each_cgi_count = nil
+    Dispatcher.dispatch_hook = nil
+    GC.enable
+  end
+
+  def test_normal_gc
+    @handler = RailsFCGIHandler.new(@log)
+    assert_nil @handler.gc_request_period
+
+    # When GC is enabled, GC.disable disables and returns false.
+    assert_equal false, GC.disable
+  end
+
+  def test_periodic_gc
+    Dispatcher.dispatch_hook = lambda do |cgi|
+      # When GC is disabled, GC.enable enables and returns true.
+      assert_equal true, GC.enable
+      GC.disable
+    end
+
+    @handler = RailsFCGIHandler.new(@log, 10)
+    assert_equal 10, @handler.gc_request_period
+    FCGI.each_cgi_count = 1
+    @handler.process!
+    assert_equal 1, @handler.gc_runs
+
+    FCGI.each_cgi_count = 10
+    @handler.process!
+    assert_equal 3, @handler.gc_runs
+
+    FCGI.each_cgi_count = 25
+    @handler.process!
+    assert_equal 6, @handler.gc_runs
+
+    assert_nil @handler.exit_code
+    assert_nil @handler.when_ready
+    assert !@handler.processing
+  end
+end
diff --git a/railties/test/mocks/dispatcher.rb b/railties/test/mocks/dispatcher.rb
index 9ca6b609c6..6561a13581 100644
--- a/railties/test/mocks/dispatcher.rb
+++ b/railties/test/mocks/dispatcher.rb
@@ -2,8 +2,10 @@ class Dispatcher
   class <<self
     attr_accessor :time_to_sleep
     attr_accessor :raise_exception
+    attr_accessor :dispatch_hook
 
     def dispatch(cgi)
+      dispatch_hook.call(cgi) if dispatch_hook
       sleep(time_to_sleep || 0)
       raise raise_exception, "Something died" if raise_exception
     end
diff --git a/railties/test/mocks/fcgi.rb b/railties/test/mocks/fcgi.rb
index 071b8e1848..59260a684f 100644
--- a/railties/test/mocks/fcgi.rb
+++ b/railties/test/mocks/fcgi.rb
@@ -2,11 +2,14 @@ class FCGI
   class << self
     attr_accessor :time_to_sleep
     attr_accessor :raise_exception
+    attr_accessor :each_cgi_count
 
     def each_cgi
-      sleep(time_to_sleep || 0)
-      raise raise_exception, "Something died" if raise_exception
-      yield "mock cgi value"
+      (each_cgi_count || 1).times do
+        sleep(time_to_sleep || 0)
+        raise raise_exception, "Something died" if raise_exception
+        yield "mock cgi value"
+      end
     end
   end
 end