From 5eea32764571b721cd1a89cf9ebfa853c621ac9e Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sat, 26 Dec 2009 17:04:57 -0800 Subject: exit with failure if master dies when daemonized MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This behavior change also means our grandparent (launched from a controlling terminal or script) will wait until the master process is ready before returning. Thanks to Iñaki Baz Castillo for the initial implementations and inspiration. --- bin/unicorn | 2 +- bin/unicorn_rails | 2 +- lib/unicorn.rb | 9 +++++++-- lib/unicorn/launcher.rb | 37 +++++++++++++++++++++++++++++-------- test/exec/test_exec.rb | 2 +- 5 files changed, 39 insertions(+), 13 deletions(-) diff --git a/bin/unicorn b/bin/unicorn index 651c2ff..5af021d 100755 --- a/bin/unicorn +++ b/bin/unicorn @@ -161,5 +161,5 @@ if $DEBUG }) end -Unicorn::Launcher.daemonize! if daemonize +Unicorn::Launcher.daemonize!(options) if daemonize Unicorn.run(app, options) diff --git a/bin/unicorn_rails b/bin/unicorn_rails index 4a22a8c..b1458fc 100755 --- a/bin/unicorn_rails +++ b/bin/unicorn_rails @@ -202,6 +202,6 @@ end if daemonize options[:pid] = rails_pid - Unicorn::Launcher.daemonize! + Unicorn::Launcher.daemonize!(options) end Unicorn.run(app, options) diff --git a/lib/unicorn.rb b/lib/unicorn.rb index 71d5994..ae05f03 100644 --- a/lib/unicorn.rb +++ b/lib/unicorn.rb @@ -25,7 +25,8 @@ module Unicorn class << self def run(app, options = {}) - HttpServer.new(app, options).start.join + ready_pipe = options.delete(:ready_pipe) + HttpServer.new(app, options).start.join(ready_pipe) end end @@ -313,7 +314,7 @@ module Unicorn # (or until a termination signal is sent). This handles signals # one-at-a-time time and we'll happily drop signals in case somebody # is signalling us too often. - def join + def join(ready_pipe = nil) # this pipe is used to wake us up from select(2) in #join when signals # are trapped. See trap_deferred init_self_pipe! @@ -324,6 +325,10 @@ module Unicorn trap(:CHLD) { |sig_nr| awaken_master } proc_name 'master' logger.info "master process ready" # test_exec.rb relies on this message + if ready_pipe + ready_pipe.syswrite($$.to_s) + ready_pipe.close + end begin loop do reap_all_workers diff --git a/lib/unicorn/launcher.rb b/lib/unicorn/launcher.rb index 1229b84..2d6ad97 100644 --- a/lib/unicorn/launcher.rb +++ b/lib/unicorn/launcher.rb @@ -19,21 +19,42 @@ class Unicorn::Launcher # 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. - def self.daemonize! + def self.daemonize!(options = {}) $stdin.reopen("/dev/null") + $stdin.sync = true # may not do anything... # We only start a new process group if we're not being reexecuted # and inheriting file descriptors from our parent unless ENV['UNICORN_FD'] - exit if fork - Process.setsid - exit if fork + # grandparent - reads pipe, exits when master is ready + # \_ parent - exits immediately ASAP + # \_ unicorn master - writes to pipe when ready - # $stderr/$stderr can/will be redirected separately in the Unicorn config - Unicorn::Configurator::DEFAULTS[:stderr_path] = "/dev/null" - Unicorn::Configurator::DEFAULTS[:stdout_path] = "/dev/null" + rd, wr = IO.pipe + grandparent = $$ + if fork + wr.close # grandparent does not write + else + rd.close # unicorn master does not read + Process.setsid + exit if fork # parent dies now + end + + if grandparent == $$ + # this will block until HttpServer#join runs (or it dies) + master_pid = rd.sysread(16).to_i + exit!(1) unless master_pid > 1 + exit 0 + else # unicorn master process + options[:ready_pipe] = wr + # $stderr/$stderr can/will be redirected separately in the + # Unicorn config + Unicorn::Configurator::DEFAULTS[:stderr_path] = "/dev/null" + Unicorn::Configurator::DEFAULTS[:stdout_path] = "/dev/null" + + # returns so Unicorn.run can happen + end end - $stdin.sync = $stdout.sync = $stderr.sync = true end end diff --git a/test/exec/test_exec.rb b/test/exec/test_exec.rb index fc0719b..24ba856 100644 --- a/test/exec/test_exec.rb +++ b/test/exec/test_exec.rb @@ -805,7 +805,7 @@ EOF exec($unicorn_bin, "-D", "-l#{@addr}:#{@port}", "-c#{ucfg.path}") end pid, status = Process.waitpid2(pid) - assert status.success?, "original process exited successfully" + assert ! status.success?, "original process exited successfully" sleep 1 # can't waitpid on a daemonized process :< assert err.stat.size > 0 end -- cgit v1.2.3-24-ge0c7 From 36888441021fbb0ae0cf724dc4e700d316b4d1bd Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Sun, 27 Dec 2009 19:38:39 -0800 Subject: Avoid leaking ready pipe file descriptor to workers Otherwise the original spawner process may not notice the close as it's still being shared by workers. While we're at it, avoid confusing the original spawner by using readpartial instead of sysread. --- lib/unicorn.rb | 17 +++++++++++------ lib/unicorn/launcher.rb | 2 +- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/unicorn.rb b/lib/unicorn.rb index ae05f03..114ef9d 100644 --- a/lib/unicorn.rb +++ b/lib/unicorn.rb @@ -25,8 +25,7 @@ module Unicorn class << self def run(app, options = {}) - ready_pipe = options.delete(:ready_pipe) - HttpServer.new(app, options).start.join(ready_pipe) + HttpServer.new(app, options).start.join end end @@ -39,7 +38,7 @@ module Unicorn :before_fork, :after_fork, :before_exec, :logger, :pid, :app, :preload_app, :reexec_pid, :orig_app, :init_listeners, - :master_pid, :config) + :master_pid, :config, :ready_pipe) include ::Unicorn::SocketHelper # prevents IO objects in here from being GC-ed @@ -163,6 +162,7 @@ module Unicorn def initialize(app, options = {}) self.app = app self.reexec_pid = 0 + self.ready_pipe = options.delete(:ready_pipe) self.init_listeners = options[:listeners] ? options[:listeners].dup : [] self.config = Configurator.new(options.merge(:use_defaults => true)) self.listener_opts = {} @@ -314,7 +314,7 @@ module Unicorn # (or until a termination signal is sent). This handles signals # one-at-a-time time and we'll happily drop signals in case somebody # is signalling us too often. - def join(ready_pipe = nil) + def join # this pipe is used to wake us up from select(2) in #join when signals # are trapped. See trap_deferred init_self_pipe! @@ -327,7 +327,8 @@ module Unicorn logger.info "master process ready" # test_exec.rb relies on this message if ready_pipe ready_pipe.syswrite($$.to_s) - ready_pipe.close + ready_pipe.close rescue nil + self.ready_pipe = nil end begin loop do @@ -533,7 +534,11 @@ module Unicorn WORKERS.values.include?(worker_nr) and next worker = Worker.new(worker_nr, Unicorn::Util.tmpio) before_fork.call(self, worker) - WORKERS[fork { worker_loop(worker) }] = worker + WORKERS[fork { + ready_pipe.close if ready_pipe + self.ready_pipe = nil + worker_loop(worker) + }] = worker end end diff --git a/lib/unicorn/launcher.rb b/lib/unicorn/launcher.rb index 2d6ad97..0ea836b 100644 --- a/lib/unicorn/launcher.rb +++ b/lib/unicorn/launcher.rb @@ -42,7 +42,7 @@ class Unicorn::Launcher if grandparent == $$ # this will block until HttpServer#join runs (or it dies) - master_pid = rd.sysread(16).to_i + master_pid = rd.readpartial(16).to_i exit!(1) unless master_pid > 1 exit 0 else # unicorn master process -- cgit v1.2.3-24-ge0c7 From 52eee4e424198a3c80793ee9c930fd3bb0285168 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Mon, 28 Dec 2009 11:16:00 -0800 Subject: launcher: descriptive error message on startup failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rather than erroring out with a non-descript EOFError, show a warning message telling users to check the logs instead. Reported-by: Iñaki Baz Castillo mid=200912281350.44760.ibc@aliax.net --- lib/unicorn/launcher.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/unicorn/launcher.rb b/lib/unicorn/launcher.rb index 0ea836b..1871420 100644 --- a/lib/unicorn/launcher.rb +++ b/lib/unicorn/launcher.rb @@ -42,8 +42,11 @@ class Unicorn::Launcher if grandparent == $$ # this will block until HttpServer#join runs (or it dies) - master_pid = rd.readpartial(16).to_i - exit!(1) unless master_pid > 1 + master_pid = (rd.readpartial(16) rescue nil).to_i + unless master_pid > 1 + warn "master failed to start, check stderr log for details" + exit!(1) + end exit 0 else # unicorn master process options[:ready_pipe] = wr -- cgit v1.2.3-24-ge0c7 From 4239f0618c8107545fd5babd256c89ffd82553bd Mon Sep 17 00:00:00 2001 From: Iñaki Baz Castillo Date: Mon, 28 Dec 2009 15:46:26 -0800 Subject: clarify errors when listeners fail to bind When using multiple listeners, the log messages can be potentially misleading as to which listener fails to bind: Before: INFO -- : unlinking existing socket=/tmp/unicorn.sock INFO -- : listening on addr=/tmp/unicorn.sock fd=3 unicorn/socket_helper.rb:110:in `initialize': Permission denied - bind(2) (Errno::EACCES) After: INFO -- : unlinking existing socket=/tmp/openxdms.sock INFO -- : listening on addr=/tmp/openxdms.sock fd=3 FATAL -- : error adding listener addr=0.0.0.0:84 unicorn/socket_helper.rb:110:in `initialize': Permission denied - bind(2) (Errno::EACCES) --- lib/unicorn.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/unicorn.rb b/lib/unicorn.rb index 114ef9d..69ecf33 100644 --- a/lib/unicorn.rb +++ b/lib/unicorn.rb @@ -307,6 +307,9 @@ module Unicorn "(#{tries < 0 ? 'infinite' : tries} tries left)" sleep(delay) retry + rescue => err + logger.fatal "error adding listener addr=#{address}" + raise err end end -- cgit v1.2.3-24-ge0c7 From 8268fea415a8c76da810f0a43e0bf63ac6c77238 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Tue, 29 Dec 2009 12:44:20 -0800 Subject: launcher: fix compatibility with other servers Rainbows! does not yet know about ready_pipe, and will probably not know about it until Unicorn 0.97.0 --- lib/unicorn/launcher.rb | 61 ++++++++++++++++++++++++++----------------------- 1 file changed, 32 insertions(+), 29 deletions(-) diff --git a/lib/unicorn/launcher.rb b/lib/unicorn/launcher.rb index 1871420..ccf9d8c 100644 --- a/lib/unicorn/launcher.rb +++ b/lib/unicorn/launcher.rb @@ -19,44 +19,47 @@ class Unicorn::Launcher # 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. - def self.daemonize!(options = {}) + def self.daemonize!(options = nil) $stdin.reopen("/dev/null") $stdin.sync = true # may not do anything... # We only start a new process group if we're not being reexecuted # and inheriting file descriptors from our parent unless ENV['UNICORN_FD'] - # grandparent - reads pipe, exits when master is ready - # \_ parent - exits immediately ASAP - # \_ unicorn master - writes to pipe when ready - - rd, wr = IO.pipe - grandparent = $$ - if fork - wr.close # grandparent does not write - else - rd.close # unicorn master does not read - Process.setsid - exit if fork # parent dies now - end + if options + # grandparent - reads pipe, exits when master is ready + # \_ parent - exits immediately ASAP + # \_ unicorn master - writes to pipe when ready - if grandparent == $$ - # this will block until HttpServer#join runs (or it dies) - master_pid = (rd.readpartial(16) rescue nil).to_i - unless master_pid > 1 - warn "master failed to start, check stderr log for details" - exit!(1) + rd, wr = IO.pipe + grandparent = $$ + if fork + wr.close # grandparent does not write + else + rd.close # unicorn master does not read + Process.setsid + exit if fork # parent dies now end - exit 0 - else # unicorn master process - options[:ready_pipe] = wr - # $stderr/$stderr can/will be redirected separately in the - # Unicorn config - Unicorn::Configurator::DEFAULTS[:stderr_path] = "/dev/null" - Unicorn::Configurator::DEFAULTS[:stdout_path] = "/dev/null" - - # returns so Unicorn.run can happen + + if grandparent == $$ + # this will block until HttpServer#join runs (or it dies) + master_pid = (rd.readpartial(16) rescue nil).to_i + unless master_pid > 1 + warn "master failed to start, check stderr log for details" + exit!(1) + end + exit 0 + else # unicorn master process + options[:ready_pipe] = wr + end + else # backwards compat + exit if fork + Process.setsid + exit if fork end + # $stderr/$stderr can/will be redirected separately in the Unicorn config + Unicorn::Configurator::DEFAULTS[:stderr_path] = "/dev/null" + Unicorn::Configurator::DEFAULTS[:stdout_path] = "/dev/null" end end -- cgit v1.2.3-24-ge0c7 From fe005f50efc8db5b9f4b2387b3b2c42f12d7c2c0 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Tue, 29 Dec 2009 21:21:47 -0800 Subject: launcher: no point in sync-ing $stdin Inspection of the MRI source reveals that IO#sync=true only appears to only apply for writes. Though it could eventually make sense to disable read buffering by setting IO#sync=true, it does not appear to happen. Of course we never read from $stdin anyways.... --- lib/unicorn/launcher.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/unicorn/launcher.rb b/lib/unicorn/launcher.rb index ccf9d8c..e71f93b 100644 --- a/lib/unicorn/launcher.rb +++ b/lib/unicorn/launcher.rb @@ -1,6 +1,6 @@ # -*- encoding: binary -*- -$stdin.sync = $stdout.sync = $stderr.sync = true +$stdout.sync = $stderr.sync = true $stdin.binmode $stdout.binmode $stderr.binmode @@ -21,7 +21,6 @@ class Unicorn::Launcher # is a symlink or otherwise got replaced. def self.daemonize!(options = nil) $stdin.reopen("/dev/null") - $stdin.sync = true # may not do anything... # We only start a new process group if we're not being reexecuted # and inheriting file descriptors from our parent -- cgit v1.2.3-24-ge0c7