Rev talk - Oct 2009 - [PATCH 0/2] listener error handling cleanups

I''ve been hitting EAGAIN (first patch) a lot in testing, I
don''t think I''ve hit
ECONNABORTED yet but I know it''s certainly possible.

Eric Wong (2):
      listener: remove warning about spuriously readable sockets
      listener: ignore ECONNABORTED on accept()

Both of these patches are also pushed to git://yhbt.net/rev

 lib/rev/listener.rb |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)
-- 
Eric Wong

Non-blocking accept() calls to a listen socket shared between
multiple processes is prone to (harmless) EAGAIN errors.  This
is because kernels can wakeup all sleeping processes sharing
that listen socket to accept() and only one of the thundering
herd of will be successful in accept()-ing the client.
---
 lib/rev/listener.rb |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/lib/rev/listener.rb b/lib/rev/listener.rb
index 7069737..d5d02a7 100644
--- a/lib/rev/listener.rb
+++ b/lib/rev/listener.rb
@@ -40,7 +40,6 @@ module Rev
       begin
         on_connection @listen_socket.accept_nonblock
       rescue Errno::EAGAIN
-        STDERR.puts "warning: listener socket spuriously readable"
       end
     end
   end
-- 
Eric Wong

ECONNABORTED can happen if a client resets/closes the socket
before the accept() has been called by the receiver.  These are
less common in most modern servers with accept filters, but
still possible.  Since these errors are harmless like EAGAIN,
avoid propagating them up the stack.
---
 lib/rev/listener.rb |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/lib/rev/listener.rb b/lib/rev/listener.rb
index d5d02a7..a9ac5b6 100644
--- a/lib/rev/listener.rb
+++ b/lib/rev/listener.rb
@@ -39,7 +39,7 @@ module Rev
     def on_readable
       begin
         on_connection @listen_socket.accept_nonblock
-      rescue Errno::EAGAIN
+      rescue Errno::EAGAIN, Errno::ECONNABORTED
       end
     end
   end
-- 
Eric Wong

Tony Arcieri <tony at medioh.com> wrote:
> Can you include the rationale you''ve provided for swallowing these
> exceptions in the comments?
Yup, here it is (also pushed to git://yhbt.net/rev):
>From 79e2768ece2473c38d1c4cda27e1b994cf2f2c51 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson at yhbt.net>
Date: Mon, 5 Oct 2009 11:22:51 -0700
Subject: [PATCH] listener: document rationale for EAGAIN/ECONNABORTED handling

---
 lib/rev/listener.rb |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/lib/rev/listener.rb b/lib/rev/listener.rb
index a9ac5b6..c4795c1 100644
--- a/lib/rev/listener.rb
+++ b/lib/rev/listener.rb
@@ -40,6 +40,15 @@ module Rev
       begin
         on_connection @listen_socket.accept_nonblock
       rescue Errno::EAGAIN, Errno::ECONNABORTED
+        # EAGAIN can be triggered here if the socket is shared between
+        # multiple processes and a thundering herd is woken up to accept
+        # one connection, only one process will get the connection and
+        # the others will be awoken.
+        # ECONNABORTED is documented in accept() manpages but modern TCP
+        # stacks with syncookies and/or accept()-filtering for DoS
+        # protection do not see it.  In any case this error is harmless
+        # and we should instead spend our time with clients that follow
+        # through on connection attempts.
       end
     end
   end
-- 
Eric Wong

Tony Arcieri <tony at medioh.com> wrote:
> Would you mind updating the 0.3.1 section of the CHANGES file to reflect
> what you''ve done?
Not at all, done and also pushed out to git://yhbt.net/rev
>From 3dc8dcaa479695824a47b41c800ee41b0dc6056c Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson at yhbt.net>
Date: Mon, 5 Oct 2009 15:10:37 -0700
Subject: [PATCH] update CHANGES with the latest Rev::Listener changes

---
 CHANGES |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/CHANGES b/CHANGES
index 67be935..39d9668 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,10 @@
 0.3.1:
 
-*
+* Removed warning about spuriously readable sockets from Rev::Listener
+
+* Rev::Listener ignores ECONNABORTED from accept_nonblock
+
+* Document rationale for EAGAIN/ECONNABORTED handling in Rev::Listener
 
 0.3.0:
 
-- 
Eric Wong