From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <e@yhbt.net>
X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net
X-Spam-Level: 
X-Spam-ASN:  
X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,AWL,BAYES_00
	shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2
Received: from localhost (dcvr.yhbt.net [127.0.0.1])
	by dcvr.yhbt.net (Postfix) with ESMTP id F3E931F751;
	Thu, 16 Apr 2020 06:59:17 +0000 (UTC)
Date: Thu, 16 Apr 2020 06:59:17 +0000
From: Eric Wong <e@yhbt.net>
To: Stan Hu <stanhu@gmail.com>
Cc: unicorn-public@yhbt.net
Subject: Re: Sustained queuing on one listener can block requests from other
 listeners
Message-ID: <20200416065917.GA7256@dcvr>
References: <CAMBWrQ=Yh42MPtzJCEO7XryVknDNetRMuA87irWfqVuLdJmiBQ@mail.gmail.com>
 <20200415052623.GA24409@dcvr>
 <CAMBWrQmzXYjDobExF9GL1BkHAc=ibvMv1ciaSv_fLQ36MyZW0g@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <CAMBWrQmzXYjDobExF9GL1BkHAc=ibvMv1ciaSv_fLQ36MyZW0g@mail.gmail.com>
List-Id: <unicorn-public.yhbt.net>

Stan Hu <stanhu@gmail.com> wrote:
> Thanks, Eric. That patch didn't work; it spun the CPU. I think this worked?

Oops, sorry.  I was too eager to drop `nr += 1' :x

Btw, please don't top post.  Fwiw, I wouldn't mind if we stopped
quoting at all on publically-archived lists (saves space and
bandwidth).

> +++ b/lib/unicorn/http_server.rb
> @@ -708,7 +708,7 @@ def worker_loop(worker)
>        # we're probably reasonably busy, so avoid calling select()
>        # and do a speculative non-blocking accept() on ready listeners
>        # before we sleep again in select().
> -      unless nr == 0
> +      if nr == readers.size
>          tmp = ready.dup
>          redo
>        end

Your patch looks close.  However the `readers' array gets
dropped on SIGQUIT with `nuke_listeners!', so `readers.size'
is unstable.

How about this?

diff --git a/lib/unicorn/http_server.rb b/lib/unicorn/http_server.rb
index a52931a..45a2e97 100644
--- a/lib/unicorn/http_server.rb
+++ b/lib/unicorn/http_server.rb
@@ -686,6 +686,7 @@ def worker_loop(worker)
     trap(:USR1) { nr = -65536 }
 
     ready = readers.dup
+    nr_listeners = readers.size
     @after_worker_ready.call(self, worker)
 
     begin
@@ -708,7 +709,7 @@ def worker_loop(worker)
       # we're probably reasonably busy, so avoid calling select()
       # and do a speculative non-blocking accept() on ready listeners
       # before we sleep again in select().
-      unless nr == 0
+      if nr == nr_listeners
         tmp = ready.dup
         redo
       end

Thanks