From 2a8c4bea2c39d0a551feb79cb471171cf96a55db Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
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 <<EOF
+after_fork do |server, worker|
+  # test script will block while reading from $fifo,
+  # so notify the script on the first worker we spawn
+  # by opening the FIFO
+  if worker.nr == 0
+    File.open("$fifo", "wb") { |fp| fp.syswrite "START" }
+  end
+end
+EOF
+	unicorn -D -c $unicorn_config pid.ru
+	worker_wait_start
+	orig_master_pid=$unicorn_pid
+}
+
+t_begin "read original worker pid" && {
+	orig_worker_pid=$(curl -sSf http://$listen/)
+	test -n "$orig_worker_pid" && kill -0 $orig_worker_pid
+}
+
+t_begin "upgrade to new master" && {
+	kill -USR2 $orig_master_pid
+}
+
+t_begin "kill old worker" && {
+	kill -WINCH $orig_master_pid
+}
+
+t_begin "wait for new worker to start" && {
+	worker_wait_start
+	test $unicorn_pid -ne $orig_master_pid
+	new_master_pid=$unicorn_pid
+}
+
+t_begin "old master pid is stashed in $pid.oldbin" && {
+	test -s "$pid.oldbin"
+	test $orig_master_pid -eq $(cat $pid.oldbin)
+}
+
+t_begin "ensure old worker is no longer running" && {
+	i=0
+	while kill -0 $orig_worker_pid 2>/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