From 40fadd79cee6a26fc124ad42297754b88d5c59e5 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Mon, 2 Mar 2009 19:27:44 -0800
Subject: Allow stderr_path and stdout_path to be set in the config

As opposed to doing this in the shell, this allows the files to
be reopened reliably after rotation.

While we're at it, use $stderr/$stdout instead of STDERR/STDOUT
since they seem to be more favored.
---
 bin/unicorn                 | 27 +++++++++++++++++----------
 lib/unicorn.rb              |  3 +++
 lib/unicorn/configurator.rb | 34 +++++++++++++++++++++++++---------
 lib/unicorn/http_request.rb |  2 +-
 4 files changed, 46 insertions(+), 20 deletions(-)

diff --git a/bin/unicorn b/bin/unicorn
index 1ea3d1f..18edf43 100755
--- a/bin/unicorn
+++ b/bin/unicorn
@@ -1,5 +1,5 @@
 #!/home/ew/bin/ruby
-STDIN.sync = STDOUT.sync = STDERR.sync = true
+$stdin.sync = $stdout.sync = $stderr.sync = true
 require 'unicorn' # require this first to populate Unicorn::DEFAULT_START_CTX
 require 'rack'
 require 'optparse'
@@ -123,7 +123,7 @@ app = case env
 when "development"
   lambda do ||
     Rack::Builder.new do
-      use Rack::CommonLogger, STDERR
+      use Rack::CommonLogger, $stderr
       use Rack::ShowExceptions
       use Rack::Lint
       run inner_app.call
@@ -132,7 +132,7 @@ when "development"
 when "deployment"
   lambda do ||
     Rack::Builder.new do
-      use Rack::CommonLogger, STDERR
+      use Rack::CommonLogger, $stderr
       run inner_app.call
     end.to_app
   end
@@ -156,19 +156,26 @@ end
 
 # only daemonize if we're not inheriting file descriptors from our parent
 if daemonize
+
+  $stdin.reopen("/dev/null")
   unless ENV['UNICORN_FD']
     exit if fork
     Process.setsid
     exit if fork
   end
 
-  Dir.chdir("/") # setting options[:directory] will override this later on
-  File.umask(0000)
-  STDIN.reopen("/dev/null")
-
-  # we can redirect these again in the Unicorn {before,after}_fork hooks
-  STDOUT.reopen("/dev/null", "a")
-  STDERR.reopen("/dev/null", "a")
+  # We don't do a lot of standard daemonization stuff:
+  #   * $stderr/$stderr can/will be redirected separately
+  #   * umask is whatever was set by the parent process at startup
+  #     and can be set in config.ru and config_file, so making it
+  #     0000 and potentially exposing sensitive log data can be bad
+  #     policy.
+  #   * Don't bother to chdir here since Unicorn is designed to
+  #     run inside APP_ROOT (or whatever the "directory" directive
+  #     specifies).  Unicorn will also re-chdir() to the directory
+  #     it was started in when being re-executed to pickup code
+  #     changes if the original deployment directory is a symlink
+  #     or otherwise got replaced.
 end
 
 Unicorn.run(app, options)
diff --git a/lib/unicorn.rb b/lib/unicorn.rb
index 998f8b8..d5867b1 100644
--- a/lib/unicorn.rb
+++ b/lib/unicorn.rb
@@ -90,6 +90,9 @@ module Unicorn
       listen(Const::DEFAULT_LISTENER) if @listeners.empty?
       self.pid = @config[:pid]
       build_app! if @preload_app
+      $stderr.reopen(@stderr_path, "a") if @stderr_path
+      $stdout.reopen(@stdout_path, "a") if @stdout_path
+      $stderr.sync = $stdout.sync = true
       spawn_missing_workers
       self
     end
diff --git a/lib/unicorn/configurator.rb b/lib/unicorn/configurator.rb
index 65b29c6..a51dcd3 100644
--- a/lib/unicorn/configurator.rb
+++ b/lib/unicorn/configurator.rb
@@ -4,7 +4,7 @@ module Unicorn
   class Configurator
     include ::Unicorn::SocketHelper
 
-    DEFAULT_LOGGER = Logger.new(STDERR) unless defined?(DEFAULT_LOGGER)
+    DEFAULT_LOGGER = Logger.new($stderr) unless defined?(DEFAULT_LOGGER)
 
     DEFAULTS = {
       :timeout => 60,
@@ -24,6 +24,8 @@ module Unicorn
       :pid => nil,
       :backlog => 1024,
       :preload_app => false,
+      :stderr_path => nil,
+      :stdout_path => nil,
     }
 
     attr_reader :config_file
@@ -130,14 +132,7 @@ module Unicorn
     end
 
     # sets the +path+ for the PID file of the unicorn master process
-    def pid(path)
-      if path
-        path = File.expand_path(path)
-        File.writable?(File.dirname(path)) or raise ArgumentError,
-                                     "directory for pid=#{path} not writable"
-      end
-      @set[:pid] = path
-    end
+    def pid(path); set_path(:pid, path); end
 
     def directory(path)
       @set[:directory] = path ? File.expand_path(path) : nil
@@ -152,8 +147,29 @@ module Unicorn
       end
     end
 
+    def stderr_path(path)
+      set_path(:stderr_path, path)
+    end
+
+    def stdout_path(path)
+      set_path(:stdout_path, path)
+    end
+
     private
 
+    def set_path(var, path) #:nodoc:
+      case path
+      when NilClass
+      when String
+        path = File.expand_path(path)
+        File.writable?(File.dirname(path)) or \
+               raise ArgumentError, "directory for #{var}=#{path} not writable"
+      else
+        raise ArgumentError
+      end
+      @set[var] = path
+    end
+
     def set_hook(var, my_proc) #:nodoc:
       case my_proc
       when Proc
diff --git a/lib/unicorn/http_request.rb b/lib/unicorn/http_request.rb
index 9571754..6e909b6 100644
--- a/lib/unicorn/http_request.rb
+++ b/lib/unicorn/http_request.rb
@@ -133,7 +133,7 @@ module Unicorn
       @params.delete "HTTP_CONTENT_LENGTH"
       @params.update({ "rack.version" => [0,1],
                       "rack.input" => @body,
-                      "rack.errors" => STDERR,
+                      "rack.errors" => $stderr,
                       "rack.multithread" => false,
                       "rack.multiprocess" => true,
                       "rack.run_once" => false,
-- 
cgit v1.2.3-24-ge0c7