unicorn Ruby/Rack server user+dev discussion/patches/pulls/bugs/help
 help / color / mirror / code / Atom feed
* listen loop error in 4.8.0
@ 2014-01-27 16:56 Eric Wong
  2014-01-27 17:02 ` Eric Wong
  0 siblings, 1 reply; 2+ messages in thread
From: Eric Wong @ 2014-01-27 16:56 UTC (permalink / raw)
  To: mongrel-unicorn

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: listen loop error in 4.8.0
  2014-01-27 16:56 listen loop error in 4.8.0 Eric Wong
@ 2014-01-27 17:02 ` Eric Wong
  0 siblings, 0 replies; 2+ messages in thread
From: Eric Wong @ 2014-01-27 17:02 UTC (permalink / raw)
  To: mongrel-unicorn

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2014-01-27 17:02 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-27 16:56 listen loop error in 4.8.0 Eric Wong
2014-01-27 17:02 ` Eric Wong

Code repositories for project(s) associated with this inbox:

	../../../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).