preserve user/group ownership when reopening logs

This is only supported when SIGUSR1 is sent only to the master
process (which then resends SIGUSR1 to the workers).

Since we only added support for user/group switching in the
workers, we now chown any log files upon switching users so the
master can pick up and chown the log files later on.  Thus
we can avoid having to restart workers because they fail to
rotate log files on their own.

3 files changed, 32 insertions, 7 deletions

diff --git a/SIGNALS b/SIGNALS
index 30a0ab0..5c7295e 100644
--- a/SIGNALS
+++ b/SIGNALS
@@ -50,6 +50,13 @@ automatically respawned.
   the current request, so multiple log lines for one request
   (as done by Rails) will not be split across multiple logs.
 
+  It is NOT recommended to send the USR1 signal directly to workers via
+  "killall -USR1 unicorn" if you are using user/group-switching support
+  in your workers.  You will encounter incorrect file permissions and
+  workers will need to be respawned.  Sending USR1 to the master process
+  first will ensure logs have the correct permissions before the master
+  forwards the USR1 signal to workers.
+
 === Procedure to replace a running unicorn executable
 
 You may replace a running instance of unicorn with a new one without
diff --git a/lib/unicorn.rb b/lib/unicorn.rb
index c6c311e..1511b03 100644
--- a/lib/unicorn.rb
+++ b/lib/unicorn.rb
@@ -142,6 +142,7 @@ module Unicorn
         # capabilities.  Let the caller handle any and all errors.
         uid = Etc.getpwnam(user).uid
         gid = Etc.getgrnam(group).gid if group
+        Unicorn::Util.chown_logs(uid, gid)
         tmp.chown(uid, gid)
         if gid && Process.egid != gid
           Process.initgroups(user, gid)
diff --git a/lib/unicorn/util.rb b/lib/unicorn/util.rb
index 6444699..3951596 100644
--- a/lib/unicorn/util.rb
+++ b/lib/unicorn/util.rb
@@ -17,6 +17,22 @@ module Unicorn
   class Util
     class << self
 
+      def is_log?(fp)
+        append_flags = File::WRONLY | File::APPEND
+
+        ! fp.closed? &&
+          fp.sync &&
+          fp.path[0] == ?/ &&
+          (fp.fcntl(Fcntl::F_GETFL) & append_flags) == append_flags
+      end
+
+      def chown_logs(uid, gid)
+        ObjectSpace.each_object(File) do |fp|
+          is_log?(fp) or next
+          fp.chown(uid, gid)
+        end
+      end
+
       # This reopens ALL logfiles in the process that have been rotated
       # using logrotate(8) (without copytruncate) or similar tools.
       # A +File+ object is considered for reopening if it is:
@@ -27,16 +43,13 @@ module Unicorn
       # Returns the number of files reopened
       def reopen_logs
         nr = 0
-        append_flags = File::WRONLY | File::APPEND
 
         ObjectSpace.each_object(File) do |fp|
-          next if fp.closed?
-          next unless (fp.sync && fp.path[0] == ?/)
-          next unless (fp.fcntl(Fcntl::F_GETFL) & append_flags) == append_flags
-
+          is_log?(fp) or next
+          orig_st = fp.stat
           begin
-            a, b = fp.stat, File.stat(fp.path)
-            next if a.ino == b.ino && a.dev == b.dev
+            b = File.stat(fp.path)
+            next if orig_st.ino == b.ino && orig_st.dev == b.dev
           rescue Errno::ENOENT
           end
 
@@ -47,6 +60,10 @@ module Unicorn
           end
           fp.reopen(fp.path, open_arg)
           fp.sync = true
+          new_st = fp.stat
+          if orig_st.uid != new_st.uid || orig_st.gid != new_st.gid
+            fp.chown(orig_st.uid, orig_st.gid)
+          end
           nr += 1
         end # each_object
         nr