Instant shutdown signals really mean instant shutdown

Use SIGQUIT if you're going to be nice and do graceful
shutdowns.  Sometimes people run real applications on this
server and SIGINT/SIGTERM get lost/trapped when Object is
rescued and that is not good.  Also make sure we break out of
the loop properly when the master is dead.

Testcases added for both SIGINT and dead master handling.

2 files changed, 54 insertions, 4 deletions

diff --git a/lib/unicorn.rb b/lib/unicorn.rb
index db25223..a539960 100644
--- a/lib/unicorn.rb
+++ b/lib/unicorn.rb
@@ -474,7 +474,7 @@ module Unicorn
       # closing anything we IO.select on will raise EBADF
       trap(:USR1) { nr = -65536; SELF_PIPE.first.close rescue nil }
       trap(:QUIT) { alive = nil; LISTENERS.each { |s| s.close rescue nil } }
-      [:TERM, :INT].each { |sig| trap(sig) { exit(0) } } # instant shutdown
+      [:TERM, :INT].each { |sig| trap(sig) { exit!(0) } } # instant shutdown
       @logger.info "worker=#{worker.nr} ready"
 
       while alive
@@ -512,7 +512,7 @@ module Unicorn
           # and do a speculative accept_nonblock on every listener
           # before we sleep again in select().
           if nr == 0 # (nr < 0) => reopen logs
-            master_pid == Process.ppid or exit(0)
+            master_pid == Process.ppid or return
             alive.chmod(nr += 1)
             begin
               # timeout used so we can detect parent death:
@@ -524,8 +524,6 @@ module Unicorn
               nr < 0 or exit(alive ? 1 : 0)
             end
           end
-        rescue SignalException, SystemExit => e
-          raise e
         rescue Object => e
           if alive
             logger.error "Unhandled listen loop exception #{e.inspect}."
diff --git a/test/unit/test_signals.rb b/test/unit/test_signals.rb
index d8af285..ca81d3e 100644
--- a/test/unit/test_signals.rb
+++ b/test/unit/test_signals.rb
@@ -37,6 +37,58 @@ class SignalsTest < Test::Unit::TestCase
     @server = nil
   end
 
+  def test_worker_dies_on_dead_master
+    pid = fork {
+      app = lambda { |env| [ 200, {'X-Pid' => "#$$" }, [] ] }
+      opts = @server_opts.merge(:timeout => 2)
+      redirect_test_io { HttpServer.new(app, opts).start.join }
+    }
+    child = sock = buf = t0 = nil
+    assert_nothing_raised do
+      wait_master_ready("test_stderr.#{pid}.log")
+      sock = TCPSocket.new('127.0.0.1', @port)
+      sock.syswrite("GET / HTTP/1.0\r\n\r\n")
+      buf = sock.readpartial(4096)
+      sock.close
+      buf =~ /\bX-Pid: (\d+)\b/ or raise Exception
+      child = $1.to_i
+      Process.kill(:KILL, pid)
+      Process.waitpid(pid)
+      t0 = Time.now
+    end
+    assert child
+    assert t0
+    assert_raises(Errno::ESRCH) { loop { Process.kill(0, child); sleep 0.2 } }
+    assert((Time.now - t0) < 60)
+  end
+
+  def test_sleepy_kill
+    rd, wr = IO.pipe
+    pid = fork {
+      rd.close
+      app = lambda { |env| wr.syswrite('.'); sleep; [ 200, {}, [] ] }
+      redirect_test_io { HttpServer.new(app, @server_opts).start.join }
+    }
+    sock = buf = nil
+    wr.close
+    assert_nothing_raised do
+      sock = TCPSocket.new('127.0.0.1', @port)
+      sock.syswrite("GET / HTTP/1.0\r\n\r\n")
+      buf = rd.readpartial(1)
+      wait_master_ready("test_stderr.#{pid}.log")
+      Process.kill(:INT, pid)
+      Process.waitpid(pid)
+    end
+    assert_equal '.', buf
+    buf = nil
+    assert_raises(EOFError,Errno::ECONNRESET,Errno::EPIPE,Errno::EINVAL,
+                  Errno::EBADF) do
+      buf = sock.sysread(4096)
+    end
+    assert_nil buf
+    ensure
+  end
+
   def test_timeout_slow_response
     pid = fork {
       app = lambda { |env| sleep }