listen loop error in 4.8.0

listen loop error in 4.8.0

[Eric Wong]

Hey all, I'm trying to diagnose an issue with another user over
private email.

It seems io.close in the trap(:QUIT) handler of the worker process is
causing an IOError, which means an IO in the readers array already got
closed somehow.  This shouldn't happen, and CRuby 2.x doesn't seem
to interrupt itself inside signal handlers[1].

Below is what I have so far...

Worst case is we're not any worse off than before; but we
could be hiding another bug with the "rescue nil" on io.close.

An extra set of eyes would be appreciated.

Pushed to the llerrloop branch of git://bogomips.org/unicorn.git
Also on rubygems.org: gem install --pre -v 4.8.0.1.g10a2 unicorn

[1] - however other Ruby runtimes may.

Subject: [PATCH] http_server: safer SIGQUIT handler for worker

This protects us from two potential errors:

1) we (or our app) somehow called IO#close on one of the sockets
   we listen on without removing it from the readers array.
   We'll ignore IOErrors from IO#close and assume we wanted to
   close it.

2) our SIGQUIT handler is interrupted by itself.  This is currently
   not possible with (MRI) Ruby 2.x, but it may happen in other
   implementations (as it does in normal C code).
---
 lib/unicorn/http_server.rb | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb
index ae8ad13..2052d53 100644
--- a/lib/unicorn/http_server.rb
+++ b/lib/unicorn/http_server.rb
@@ -591,6 +591,13 @@ class Unicorn::HttpServer
   EXIT_SIGS = [ :QUIT, :TERM, :INT ]
   WORKER_QUEUE_SIGS = QUEUE_SIGS - EXIT_SIGS
 
+  def nuke_listeners!(readers)
+    # only called from the worker, ordering is important here
+    tmp = readers.dup
+    readers.replace([false]) # ensure worker does not continue ASAP
+    tmp.each { |io| io.close rescue nil } # break out of IO.select
+  end
+
   # gets rid of stuff the worker has no business keeping track of
   # to free some resources and drops all sig handlers.
   # traps for USR1, USR2, and HUP may be set in the after_fork Proc
@@ -618,7 +625,7 @@ class Unicorn::HttpServer
     @after_fork = @listener_opts = @orig_app = nil
     readers = LISTENERS.dup
     readers << worker
-    trap(:QUIT) { readers.each { |io| io.close }.replace([false]) }
+    trap(:QUIT) { nuke_listeners!(readers) }
     readers
   end
 
@@ -677,7 +684,7 @@ class Unicorn::HttpServer
       worker.tick = Time.now.to_i
       ret = IO.select(readers, nil, nil, @timeout) and ready = ret[0]
     rescue => e
-      redo if nr < 0
+      redo if nr < 0 && readers[0]
       Unicorn.log_error(@logger, "listen loop error", e) if readers[0]
     end while readers[0]
   end
-- 
1.8.5.3.368.gab0bcec
_______________________________________________
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

Re: listen loop error in 4.8.0

[Eric Wong]

Eric Wong <normalperson@yhbt.net> wrote:
> It seems io.close in the trap(:QUIT) handler of the worker process is
> causing an IOError, which means an IO in the readers array already got
> closed somehow.  This shouldn't happen, and CRuby 2.x doesn't seem
> to interrupt itself inside signal handlers[1].

Of course, the fake-signal mechanism of 4.8.0 could also cause this:

* the master triggers a fake-signal (where the invoked trap handler
  is interruptible)
* a real signal also triggered from an outside user

I forget to mention this happened with unicorn-worker-killer 0.4.2 for
that user, and that seems to just use normal Process.kill
_______________________________________________
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