From aad1fdfc17e2fe1a6308690daf74456877796f51 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Thu, 8 Jul 2010 05:45:22 +0000 Subject: tee_input: safer record separator ($/) handling Different threads may change $/ during execution, so cache it at function entry to a local variable for safety. $/ may also be of a non-binary encoding, so rely on Rack::Utils.bytesize to portably capture the correct size. Our string slicing is always safe from 1.9 encoding: both our socket and backing temporary file are opened in binary mode, so we'll always be dealing with binary strings in this class (in accordance to the Rack spec). (cherry picked from commit 1cd698f8c7938b1f19e9ba091708cb4515187939) --- lib/unicorn/tee_input.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/unicorn/tee_input.rb b/lib/unicorn/tee_input.rb index 8ff7258..563747c 100644 --- a/lib/unicorn/tee_input.rb +++ b/lib/unicorn/tee_input.rb @@ -109,7 +109,7 @@ module Unicorn # unlike IO#gets. def gets socket or return tmp.gets - nil == $/ and return read + sep = $/ or return read orig_size = tmp.size if tmp.pos == orig_size @@ -117,8 +117,9 @@ module Unicorn tmp.seek(orig_size) end + sep_size = Rack::Utils.bytesize(sep) line = tmp.gets # cannot be nil here since size > pos - $/ == line[-$/.size, $/.size] and return line + sep == line[-sep_size, sep_size] and return line # unlikely, if we got here, then tmp is at EOF begin @@ -126,7 +127,7 @@ module Unicorn tee(Const::CHUNK_SIZE, buf2) or break tmp.seek(orig_size) line << tmp.gets - $/ == line[-$/.size, $/.size] and return line + sep == line[-sep_size, sep_size] and return line # tmp is at EOF again here, retry the loop end while true -- cgit v1.2.3-24-ge0c7 From ae1f5e2d331d1714dd1b71d4905b296abf7780d0 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Tue, 13 Jul 2010 08:53:48 +0000 Subject: launcher: do not re-daemonize when USR2 upgrading This was accidentally enabled when ready_pipe was developed. While re-daemonizing appears harmless in most cases this makes detecting backed-out upgrades from the original master process impossible. (cherry picked from commit 3f0f9d6d72cf17b34c130b86eb933bbc513b24b3) --- lib/unicorn/launcher.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/unicorn/launcher.rb b/lib/unicorn/launcher.rb index 0d957cf..0d415dd 100644 --- a/lib/unicorn/launcher.rb +++ b/lib/unicorn/launcher.rb @@ -24,11 +24,7 @@ module Unicorn::Launcher # We only start a new process group if we're not being reexecuted # and inheriting file descriptors from our parent - if ENV['UNICORN_FD'] - exit if fork - Process.setsid - exit if fork - else + unless ENV['UNICORN_FD'] # grandparent - reads pipe, exits when master is ready # \_ parent - exits immediately ASAP # \_ unicorn master - writes to pipe when ready -- cgit v1.2.3-24-ge0c7 From 2a8c4bea2c39d0a551feb79cb471171cf96a55db Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Tue, 13 Jul 2010 08:57:37 +0000 Subject: SIGHUP deals w/ dual master pid path scenario As described in our SIGNALS documentation, sending SIGHUP to the old master (to respawn SIGWINCH-ed children) while the new master (spawned from SIGUSR2) is active is useful for backing out of an upgrade before sending SIGQUIT to the new master. Unfortunately, the SIGHUP signal to the old master will cause the ".oldbin" pid file to be reset to the non-".oldbin" version and thus attempt to clobber the pid file in use by the to-be-terminated new master process. Thanks to the previous commit to prevent redaemonization in the new master, the old master can reliably detect if the new master is active while it is reloading the config file. Thanks to Lawrence Pit for discovering this bug. ref: http://mid.gmane.org/4C3BEACF.7040301@gmail.com (cherry picked from commit c13bec3449396b21795966101367838161612d61) --- lib/unicorn.rb | 5 ++ t/pid.ru | 3 ++ t/t0008-back_out_of_upgrade.sh | 110 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 118 insertions(+) create mode 100644 t/pid.ru create mode 100755 t/t0008-back_out_of_upgrade.sh diff --git a/lib/unicorn.rb b/lib/unicorn.rb index a7b0646..cbb5520 100644 --- a/lib/unicorn.rb +++ b/lib/unicorn.rb @@ -312,6 +312,11 @@ module Unicorn if path if x = valid_pid?(path) return path if pid && path == pid && x == $$ + if x == reexec_pid && pid =~ /\.oldbin\z/ + logger.warn("will not set pid=#{path} while reexec-ed "\ + "child is running PID:#{x}") + return + end raise ArgumentError, "Already running on PID:#{x} " \ "(or pid=#{path} is stale)" end diff --git a/t/pid.ru b/t/pid.ru new file mode 100644 index 0000000..f5fd31f --- /dev/null +++ b/t/pid.ru @@ -0,0 +1,3 @@ +use Rack::ContentLength +use Rack::ContentType, "text/plain" +run lambda { |env| [ 200, {}, [ "#$$\n" ] ] } diff --git a/t/t0008-back_out_of_upgrade.sh b/t/t0008-back_out_of_upgrade.sh new file mode 100755 index 0000000..96d4057 --- /dev/null +++ b/t/t0008-back_out_of_upgrade.sh @@ -0,0 +1,110 @@ +#!/bin/sh +. ./test-lib.sh +t_plan 13 "backout of USR2 upgrade" + +worker_wait_start () { + test xSTART = x"$(cat $fifo)" + unicorn_pid=$(cat $pid) +} + +t_begin "setup and start" && { + unicorn_setup + rm -f $pid.oldbin + +cat >> $unicorn_config </dev/null + do + i=$(( $i + 1 )) + test $i -lt 600 || die "timed out" + sleep 1 + done +} + +t_begin "capture pid of new worker" && { + new_worker_pid=$(curl -sSf http://$listen/) +} + +t_begin "reload old master process" && { + kill -HUP $orig_master_pid + worker_wait_start +} + +t_begin "gracefully kill new master and ensure it dies" && { + kill -QUIT $new_master_pid + i=0 + while kill -0 $new_worker_pid 2>/dev/null + do + i=$(( $i + 1 )) + test $i -lt 600 || die "timed out" + sleep 1 + done +} + +t_begin "ensure $pid.oldbin does not exist" && { + i=0 + while test -s $pid.oldbin + do + i=$(( $i + 1 )) + test $i -lt 600 || die "timed out" + sleep 1 + done + while ! test -s $pid + do + i=$(( $i + 1 )) + test $i -lt 600 || die "timed out" + sleep 1 + done +} + +t_begin "ensure $pid is correct" && { + cur_master_pid=$(cat $pid) + test $orig_master_pid -eq $cur_master_pid +} + +t_begin "killing succeeds" && { + kill $orig_master_pid +} + +dbgcat r_err + +t_done -- cgit v1.2.3-24-ge0c7 From a965c0bb48d5b92373f939865212641d810c97d7 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Tue, 13 Jul 2010 12:54:26 -0700 Subject: unicorn 1.0.1 - bugfixes only The first maintenance release of 1.0.x, this release is primarily to fix a long-standing bug where the original PID file is not restored when rolling back from a USR2 upgrade. Presumably most upgrades aren't rolled back, so it took over a year to notice this issue. Thanks to Lawrence Pit for discovering and reporting this issue. There is also a pedantic TeeInput bugfix which shouldn't affect real apps from the 1.1.x series and a test case fix for OSX, too. --- GIT-VERSION-GEN | 2 +- lib/unicorn/const.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index 88b943a..7d0c7ed 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v1.0.0.GIT +DEF_VER=v1.0.1.GIT LF=' ' diff --git a/lib/unicorn/const.rb b/lib/unicorn/const.rb index d3ccead..51a8a3b 100644 --- a/lib/unicorn/const.rb +++ b/lib/unicorn/const.rb @@ -8,8 +8,8 @@ module Unicorn # Symbols did not really improve things much compared to constants. module Const - # The current version of Unicorn, currently 1.0.0 - UNICORN_VERSION="1.0.0" + # The current version of Unicorn, currently 1.0.1 + UNICORN_VERSION="1.0.1" DEFAULT_HOST = "0.0.0.0" # default TCP listen host address DEFAULT_PORT = 8080 # default TCP listen port -- cgit v1.2.3-24-ge0c7