From 72ed1ceba5dbfe3656480af22740118b9e06d418 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Fri, 27 Mar 2009 00:20:24 -0700
Subject: Deferred log rotation in workers

Instead of rotating logs immediately when SIGUSR1 is caught,
defer it until the current client is processing is complete.

This allows multi-line log messages generated by apps to not be
broken up if SIGUSR1 is received while the app is running.

If we're sleeping inside IO.select, we close a pipe in the
exceptfds set to cause EBADF to be raised.

This also adds a small reliability improvement to test_exec
so we wait until signals are ready before sending USR1
to rotate logs.
---
 lib/unicorn.rb         | 25 +++++++++++++------------
 test/exec/test_exec.rb |  2 +-
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/lib/unicorn.rb b/lib/unicorn.rb
index 5bf7519..b36ae21 100644
--- a/lib/unicorn.rb
+++ b/lib/unicorn.rb
@@ -435,19 +435,19 @@ module Unicorn
         alive = false
         @listeners.each { |sock| sock.close rescue nil } # break IO.select
       end
-      trap('USR1') do
-        begin
+      reopen_logs, (rd, wr) = false, IO.pipe
+      trap(:USR1) { reopen_logs = true; rd.close rescue nil }
+      @logger.info "worker=#{worker.nr} ready"
+
+      while alive && @master_pid == Process.ppid
+        if reopen_logs
+          reopen_logs = false
           @logger.info "worker=#{worker.nr} rotating logs..."
           Unicorn::Util.reopen_logs
           @logger.info "worker=#{worker.nr} done rotating logs"
-        rescue Object => err
-          @logger.error "error rotating logs: #{err.inspect}"
-          @logger.error "gracefully restarting worker=#{worker.nr} PID:#$$"
-          Process.kill('QUIT', $$)
+          wr.close rescue nil
+          rd, wr = IO.pipe
         end
-      end
-
-      while alive && @master_pid == Process.ppid
         # we're a goner in @timeout seconds anyways if tempfile.chmod
         # breaks, so don't trap the exception.  Using fchmod() since
         # futimes() is not available in base Ruby and I very strongly
@@ -477,6 +477,7 @@ module Unicorn
               end
             end
             tempfile.chmod(nr += 1)
+            break if reopen_logs
           end
           client = nil
 
@@ -484,18 +485,18 @@ module Unicorn
           # we're probably reasonably busy, so avoid calling select(2)
           # and try to do a blind non-blocking accept(2) on everything
           # before we sleep again in select
-          if accepted
+          if accepted || reopen_logs
             ready = @listeners
           else
             begin
               tempfile.chmod(nr += 1)
               # timeout used so we can detect parent death:
-              ret = IO.select(@listeners, nil, nil, @timeout/2.0) or next
+              ret = IO.select(@listeners, nil, [rd], @timeout/2.0) or next
               ready = ret[0]
             rescue Errno::EINTR
               ready = @listeners
             rescue Errno::EBADF => e
-              exit(alive ? 1 : 0)
+              reopen_logs or exit(alive ? 1 : 0)
             end
           end
         rescue SystemExit => e
diff --git a/test/exec/test_exec.rb b/test/exec/test_exec.rb
index ea9fc7c..53cebf6 100644
--- a/test/exec/test_exec.rb
+++ b/test/exec/test_exec.rb
@@ -502,7 +502,7 @@ end
       lines = []
       while (tries -= 1) > 0
         begin
-          lines = File.readlines(path).grep(/worker=\d+ spawned/)
+          lines = File.readlines(path).grep(/worker=\d+ ready/)
           lines.size == nr_workers and return
         rescue Errno::ENOENT
         end
-- 
cgit v1.2.3-24-ge0c7