From 9435ee2d5111394739b82d0f8a275deca8d505be Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Wed, 16 Sep 2009 23:04:37 -0700 Subject: 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 --- lib/unicorn.rb | 2 ++ test/exec/test_exec.rb | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++ test/test_helper.rb | 6 ++++ 3 files changed, 105 insertions(+) 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) -- cgit v1.2.3-24-ge0c7