From: Eric Wong <normalperson@yhbt.net>
To: unicorn list <mongrel-unicorn@rubyforge.org>
Subject: [PATCH] avoid unlinking actively listening sockets
Date: Mon, 4 Oct 2010 04:22:58 +0000 [thread overview]
Message-ID: <20101004042258.GA30932@dcvr.yhbt.net> (raw)
In-Reply-To: <20101004041713.GA28709@dcvr.yhbt.net>
While we've always unlinked dead sockets from nuked/leftover
processes, blindly unlinking them can cause unnecessary failures
when an active process is already listening on them. We now
make a simple connect(2) check to ensure the socket is not in
use before unlinking it.
Thanks to Jordan Ritter for the detailed bug report leading to
this fix.
ref: http://mid.gmane.org/8D95A44B-A098-43BE-B532-7D74BD957F31@darkridge.com
---
Eric Wong <normalperson@yhbt.net> wrote:
> A simpler check would be to use connect(2) (but not make any HTTP request)
> to see if the socket is alive. Patch coming.
s/simpler/better/
Also pushed out to "master". I guess a 1.1.4 release with this fix
only is on the way since there isn't much else to release, yet.
lib/unicorn/socket_helper.rb | 10 ++++-
t/t0011-active-unix-socket.sh | 79 +++++++++++++++++++++++++++++++++++++++
test/unit/test_socket_helper.rb | 9 ++++-
3 files changed, 95 insertions(+), 3 deletions(-)
create mode 100644 t/t0011-active-unix-socket.sh
diff --git a/lib/unicorn/socket_helper.rb b/lib/unicorn/socket_helper.rb
index 9a155e1..1d03eab 100644
--- a/lib/unicorn/socket_helper.rb
+++ b/lib/unicorn/socket_helper.rb
@@ -111,8 +111,14 @@ module Unicorn
sock = if address[0] == ?/
if File.exist?(address)
if File.socket?(address)
- logger.info "unlinking existing socket=#{address}"
- File.unlink(address)
+ begin
+ UNIXSocket.new(address).close
+ # fall through, try to bind(2) and fail with EADDRINUSE
+ # (or succeed from a small race condition we can't sanely avoid).
+ rescue Errno::ECONNREFUSED
+ logger.info "unlinking existing socket=#{address}"
+ File.unlink(address)
+ end
else
raise ArgumentError,
"socket=#{address} specified but it is not a socket!"
diff --git a/t/t0011-active-unix-socket.sh b/t/t0011-active-unix-socket.sh
new file mode 100644
index 0000000..6f9ac53
--- /dev/null
+++ b/t/t0011-active-unix-socket.sh
@@ -0,0 +1,79 @@
+#!/bin/sh
+. ./test-lib.sh
+t_plan 11 "existing UNIX domain socket check"
+
+read_pid_unix () {
+ x=$(printf 'GET / HTTP/1.0\r\n\r\n' | \
+ socat - UNIX:$unix_socket | \
+ tail -1)
+ test -n "$x"
+ y="$(expr "$x" : '\([0-9]\+\)')"
+ test x"$x" = x"$y"
+ test -n "$y"
+ echo "$y"
+}
+
+t_begin "setup and start" && {
+ rtmpfiles unix_socket unix_config
+ rm -f $unix_socket
+ unicorn_setup
+ grep -v ^listen < $unicorn_config > $unix_config
+ echo "listen '$unix_socket'" >> $unix_config
+ unicorn -D -c $unix_config pid.ru
+ unicorn_wait_start
+ orig_master_pid=$unicorn_pid
+}
+
+t_begin "get pid of worker" && {
+ worker_pid=$(read_pid_unix)
+ t_info "worker_pid=$worker_pid"
+}
+
+t_begin "fails to start with existing pid file" && {
+ rm -f $ok
+ unicorn -D -c $unix_config pid.ru || echo ok > $ok
+ test x"$(cat $ok)" = xok
+}
+
+t_begin "worker pid unchanged" && {
+ test x"$(read_pid_unix)" = x$worker_pid
+ > $r_err
+}
+
+t_begin "fails to start with listening UNIX domain socket bound" && {
+ rm $ok $pid
+ unicorn -D -c $unix_config pid.ru || echo ok > $ok
+ test x"$(cat $ok)" = xok
+ > $r_err
+}
+
+t_begin "worker pid unchanged (again)" && {
+ test x"$(read_pid_unix)" = x$worker_pid
+}
+
+t_begin "nuking the existing Unicorn succeeds" && {
+ kill -9 $unicorn_pid $worker_pid
+ while kill -0 $unicorn_pid
+ do
+ sleep 1
+ done
+ check_stderr
+}
+
+t_begin "succeeds in starting with leftover UNIX domain socket bound" && {
+ test -S $unix_socket
+ unicorn -D -c $unix_config pid.ru
+ unicorn_wait_start
+}
+
+t_begin "worker pid changed" && {
+ test x"$(read_pid_unix)" != x$worker_pid
+}
+
+t_begin "killing succeeds" && {
+ kill $unicorn_pid
+}
+
+t_begin "no errors" && check_stderr
+
+t_done
diff --git a/test/unit/test_socket_helper.rb b/test/unit/test_socket_helper.rb
index bbce359..c6d0d42 100644
--- a/test/unit/test_socket_helper.rb
+++ b/test/unit/test_socket_helper.rb
@@ -101,7 +101,14 @@ class TestSocketHelper < Test::Unit::TestCase
def test_bind_listen_unix_rebind
test_bind_listen_unix
- new_listener = bind_listen(@unix_listener_path)
+ new_listener = nil
+ assert_raises(Errno::EADDRINUSE) do
+ new_listener = bind_listen(@unix_listener_path)
+ end
+ assert_nothing_raised do
+ File.unlink(@unix_listener_path)
+ new_listener = bind_listen(@unix_listener_path)
+ end
assert UNIXServer === new_listener
assert new_listener.fileno != @unix_listener.fileno
assert_equal sock_name(new_listener), sock_name(@unix_listener)
--
Eric Wong
_______________________________________________
Unicorn mailing list - mongrel-unicorn@rubyforge.org
http://rubyforge.org/mailman/listinfo/mongrel-unicorn
Do not quote signatures (like this one) or top post when replying
prev parent reply other threads:[~2010-10-04 4:26 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-10-02 16:38 Problem with binding UNIX listeners before checking PID Jordan Ritter
2010-10-04 4:17 ` Eric Wong
2010-10-04 4:22 ` Eric Wong [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://yhbt.net/unicorn/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20101004042258.GA30932@dcvr.yhbt.net \
--to=normalperson@yhbt.net \
--cc=mongrel-unicorn@rubyforge.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://yhbt.net/unicorn.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).