SIGHUP no longer drops lone, default listener

When SIGHUP reloads the config, we didn't account for the case
where the listen socket was completely unspecified.  Thus the
default listener (0.0.0.0:8080), did not get preserved and
re-injected into the config properly.

Note that relying on the default listen or specifying listeners
on the command-line means it's /practically/ impossible to
_unbind_ those listeners with a configuration file reload.  We
also need to preserve the (unspecified) default listener across
upgrades that later result in SIGHUP, too; so the easiest way is
to inject the default listener into the command-line for
upgrades.

Many thanks to James Golick for reporting and helping me track
down the bug since this behavior is difficult to write reliable
automated tests for.

Signed-off-by: Eric Wong <normalperson@yhbt.net>

3 files changed, 105 insertions, 0 deletions

diff --git a/lib/unicorn.rb b/lib/unicorn.rb
index 4cc5c2d..0e46261 100644
--- a/lib/unicorn.rb
+++ b/lib/unicorn.rb
@@ -110,6 +110,8 @@ module Unicorn
       config_listeners -= listener_names
       if config_listeners.empty? && LISTENERS.empty?
         config_listeners << Unicorn::Const::DEFAULT_LISTEN
+        init_listeners << Unicorn::Const::DEFAULT_LISTEN
+        START_CTX[:argv] << "-l#{Unicorn::Const::DEFAULT_LISTEN}"
       end
       config_listeners.each { |addr| listen(addr) }
       raise ArgumentError, "no listeners" if LISTENERS.empty?
diff --git a/test/exec/test_exec.rb b/test/exec/test_exec.rb
index 3173d9a..e753c2d 100644
--- a/test/exec/test_exec.rb
+++ b/test/exec/test_exec.rb
@@ -1,6 +1,7 @@
 # -*- encoding: binary -*-
 
 # Copyright (c) 2009 Eric Wong
+FLOCK_PATH = File.expand_path(__FILE__)
 require 'test/test_helper'
 
 do_test = true
@@ -755,4 +756,100 @@ end
     hup_test_common(false)
   end
 
+  def test_default_listen_hup_holds_listener
+    default_listen_lock do
+      res, pid_path = default_listen_setup
+      daemon_pid = File.read(pid_path).to_i
+      assert_nothing_raised { Process.kill(:HUP, daemon_pid) }
+      wait_workers_ready("test_stderr.#$$.log", 1)
+      res2 = hit(["http://#{Unicorn::Const::DEFAULT_LISTEN}/"])
+      assert_match %r{\d+}, res2.first
+      assert res2.first != res.first
+      assert_nothing_raised { Process.kill(:QUIT, daemon_pid) }
+      wait_for_death(daemon_pid)
+    end
+  end
+
+  def test_default_listen_upgrade_holds_listener
+    default_listen_lock do
+      res, pid_path = default_listen_setup
+      daemon_pid = File.read(pid_path).to_i
+      assert_nothing_raised {
+        Process.kill(:USR2, daemon_pid)
+        wait_for_file("#{pid_path}.oldbin")
+        wait_for_file(pid_path)
+        Process.kill(:QUIT, daemon_pid)
+        wait_for_death(daemon_pid)
+      }
+      daemon_pid = File.read(pid_path).to_i
+      wait_workers_ready("test_stderr.#$$.log", 1)
+      File.truncate("test_stderr.#$$.log", 0)
+
+      res2 = hit(["http://#{Unicorn::Const::DEFAULT_LISTEN}/"])
+      assert_match %r{\d+}, res2.first
+      assert res2.first != res.first
+
+      assert_nothing_raised { Process.kill(:HUP, daemon_pid) }
+      wait_workers_ready("test_stderr.#$$.log", 1)
+      File.truncate("test_stderr.#$$.log", 0)
+      res3 = hit(["http://#{Unicorn::Const::DEFAULT_LISTEN}/"])
+      assert res2.first != res3.first
+
+      assert_nothing_raised { Process.kill(:QUIT, daemon_pid) }
+      wait_for_death(daemon_pid)
+    end
+  end
+
+  def default_listen_setup
+    File.open("config.ru", "wb") { |fp| fp.syswrite(HI.gsub("HI", '#$$')) }
+    pid_path = (tmp = Tempfile.new('pid')).path
+    tmp.close!
+    ucfg = Tempfile.new('unicorn_test_config')
+    ucfg.syswrite("pid '#{pid_path}'\n")
+    ucfg.syswrite("stderr_path 'test_stderr.#$$.log'\n")
+    ucfg.syswrite("stdout_path 'test_stdout.#$$.log'\n")
+    pid = xfork {
+      redirect_test_io { exec($unicorn_bin, "-D", "-c", ucfg.path) }
+    }
+    _, status = Process.waitpid2(pid)
+    assert status.success?
+    wait_master_ready("test_stderr.#$$.log")
+    wait_workers_ready("test_stderr.#$$.log", 1)
+    File.truncate("test_stderr.#$$.log", 0)
+    res = hit(["http://#{Unicorn::Const::DEFAULT_LISTEN}/"])
+    assert_match %r{\d+}, res.first
+    [ res, pid_path ]
+  end
+
+  # we need to flock() something to prevent these tests from running
+  def default_listen_lock(&block)
+    fp = File.open(FLOCK_PATH, "rb")
+    begin
+      fp.flock(File::LOCK_EX)
+      begin
+        TCPServer.new(Unicorn::Const::DEFAULT_HOST,
+                      Unicorn::Const::DEFAULT_PORT).close
+      rescue Errno::EADDRINUSE, Errno::EACCES
+        warn "can't bind to #{Unicorn::Const::DEFAULT_LISTEN}"
+        return false
+      end
+
+      # unused_port should never take this, but we may run an environment
+      # where tests are being run against older unicorns...
+      lock_path = "#{Dir::tmpdir}/unicorn_test." \
+                  "#{Unicorn::Const::DEFAULT_LISTEN}.lock"
+      begin
+        lock = File.open(lock_path, File::WRONLY|File::CREAT|File::EXCL, 0600)
+        yield
+      rescue Errno::EEXIST
+        lock_path = nil
+        return false
+      ensure
+        File.unlink(lock_path) if lock_path
+      end
+    ensure
+      fp.flock(File::LOCK_UN)
+    end
+  end
+
 end if do_test
diff --git a/test/test_helper.rb b/test/test_helper.rb
index d3bf46c..3a3e42f 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -104,6 +104,12 @@ def unused_port(addr = '127.0.0.1')
   begin
     begin
       port = base + rand(32768 - base)
+      if addr == Unicorn::Const::DEFAULT_HOST
+        while port == Unicorn::Const::DEFAULT_PORT
+          port = base + rand(32768 - base)
+        end
+      end
+
       sock = Socket.new(Socket::AF_INET, Socket::SOCK_STREAM, 0)
       sock.bind(Socket.pack_sockaddr_in(port, addr))
       sock.listen(5)