about summary refs log tree commit homepage
diff options
authorEric Wong <normalperson@yhbt.net>2009-05-03 16:16:48 -0700
committerEric Wong <normalperson@yhbt.net>2009-05-22 01:54:20 -0700
commitcfe5234e975b914e831907db77fee8f154950fc1 (patch)
parent476751e32b8ae454829491ede68f4a062d4cec9f (diff)
Timeouts of less than 2 seconds are unsafe due to the lack of
subsecond resolution in most POSIX filesystems.  This is the
trade-off for using a low-complexity solution for timeouts.
Since this type of timeout is a last resort; 2 seconds is not
entirely unreasonable IMNSHO.  Additionally, timing out too
aggressively can put us in a fork loop and slow down the system.

Of course, the default is 60 seconds and most people do not
bother to change it.
3 files changed, 6 insertions, 7 deletions
diff --git a/TODO b/TODO
index 204bb7d..085ef70 100644
--- a/TODO
+++ b/TODO
@@ -2,8 +2,6 @@
   * integration tests with nginx including bad client handling
-  * tests for timeout
   * manpages (why do so few Ruby executables come with proper manpages?)
 == 1.1.0
diff --git a/lib/unicorn.rb b/lib/unicorn.rb
index 54e2bc0..07925f2 100644
--- a/lib/unicorn.rb
+++ b/lib/unicorn.rb
@@ -375,9 +375,8 @@ module Unicorn
     # is stale for >@timeout seconds, then we'll kill the corresponding
     # worker.
     def murder_lazy_workers
-      now = Time.now
       WORKERS.each_pair do |pid, worker|
-        (now - worker.tempfile.ctime) <= @timeout and next
+        Time.now - worker.tempfile.ctime <= @timeout and next
         logger.error "worker=#{worker.nr} PID:#{pid} is too old, killing"
         kill_worker(:KILL, pid) # take no prisoners for @timeout violations
         worker.tempfile.close rescue nil
diff --git a/lib/unicorn/configurator.rb b/lib/unicorn/configurator.rb
index 6f49905..7724ff0 100644
--- a/lib/unicorn/configurator.rb
+++ b/lib/unicorn/configurator.rb
@@ -143,12 +143,14 @@ module Unicorn
     # handling the request/app.call/response cycle taking longer than
     # this time period will be forcibly killed (via SIGKILL).  This
     # timeout is enforced by the master process itself and not subject
-    # to the scheduling limitations by the worker process.
+    # to the scheduling limitations by the worker process.  Due the
+    # low-complexity, low-overhead implementation, timeouts of less
+    # than 2.0 seconds can be considered inaccurate and unsafe.
     def timeout(seconds)
       Numeric === seconds or raise ArgumentError,
                                   "not numeric: timeout=#{seconds.inspect}"
-      seconds > 0 or raise ArgumentError,
-                                  "not positive: timeout=#{seconds.inspect}"
+      seconds >= 2 or raise ArgumentError,
+                                  "too low: timeout=#{seconds.inspect}"
       @set[:timeout] = seconds