[PATCH] acceptor: close inherited-but-unneeded sockets

[PATCH] acceptor: close inherited-but-unneeded sockets

[Eric Wong]

When inheriting sockets from the parent via YAHNS_FD, we must close
sockets ASAP if they are unconfigured in the child.  This bug exists
in yahns (and not unicorn) because of the trickier shutdown routine
we do for blocking accept system calls to work reliably with the
threading support in mainline Ruby 2.x.  This bug would not exist
in a purely C server using blocking accept, either.
---
  I'll probably release 1.6.0 in a few hours with this and what's been
  sitting in master since the last release.  If there's more risk-averse
  users, maybe a 1.5.1 with just this patch is warranted, but I doubt
  it (abguvat nobhg lnuaf vf sbe gur evfx-nirefr pebjq :P).

  Eric Wong (15):
        README: add link to mailing list archives
        test_ssl: factor out server SSLContext creation
        doc: add design_notes document
        reduce File::Stat object allocations
        update comments about wbuf_close return values
        wbuf: lazily (re)create temporary file
        fix compatibility with unicorn.git
        skip tests requiring String#b on 1.9.3
        use the monotonic clock under Ruby 2.1+
        favor Class.new for method-less classes
        extras/proxy_pass: save memory in String#split arg
        extras/proxy_pass: do not name unused variable
        extras/proxy_pass: log exceptions leading to 502
        extras/proxy_pass: flesh out upload support + tests
        acceptor: close inherited-but-unneeded sockets

 lib/yahns/acceptor.rb |  5 ++++-
 test/test_server.rb   | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/lib/yahns/acceptor.rb b/lib/yahns/acceptor.rb
index cd9e055..0cebea2 100644
--- a/lib/yahns/acceptor.rb
+++ b/lib/yahns/acceptor.rb
@@ -19,7 +19,10 @@ module Yahns::Acceptor # :nodoc:
 
   # just keep looping this on every acceptor until the associated thread dies
   def ac_quit
-    return true unless defined?(@thrs)
+    unless defined?(@thrs) # acceptor has not started yet, freshly inherited
+      close
+      return true
+    end
     @thrs.each { |t| t[:yahns_quit] = true }
     return true if __ac_quit_done?
 
diff --git a/test/test_server.rb b/test/test_server.rb
index 69babb3..b7cb3e6 100644
--- a/test/test_server.rb
+++ b/test/test_server.rb
@@ -804,4 +804,43 @@ class TestServer < Testcase
   ensure
     quit_wait(pid)
   end
+
+  def test_inherit_too_many
+    err = @err
+    s2 = TCPServer.new(ENV["TEST_HOST"] || "127.0.0.1", 0)
+    cfg = Yahns::Config.new
+    host, port = @srv.addr[3], @srv.addr[1]
+    cfg.instance_eval do
+      ru = lambda { |_| [ 200, {'Content-Length'=>'2'}, ['HI'] ] }
+      GTL.synchronize { app(:rack, ru) { listen "#{host}:#{port}" } }
+      logger(Logger.new(err.path))
+    end
+    mkserver(cfg, @srv) do
+      s2.autoclose = false
+      ENV["YAHNS_FD"] = "#{@srv.fileno},#{s2.fileno}"
+    end
+    run_client(host, port) { |res| assert_equal "HI", res.body }
+    th = Thread.new do
+      c = s2.accept
+      c.readpartial(1234)
+      c.write "HTTP/1.0 666 OK\r\n\r\nGO AWAY"
+      c.close
+      :OK
+    end
+    Thread.pass
+    s2host, s2port = s2.addr[3], s2.addr[1]
+    Net::HTTP.start(s2host, s2port) do |http|
+      res = http.request(Net::HTTP::Get.new("/"))
+      assert_equal 666, res.code.to_i
+      assert_equal "GO AWAY", res.body
+    end
+    assert_equal :OK, th.value
+    tmpc = TCPSocket.new(s2host, s2port)
+    a2 = s2.accept
+    assert_nil IO.select([a2], nil, nil, 0.05)
+    tmpc.close
+    assert_nil a2.read(1)
+    a2.close
+    s2.close
+  end
 end
-- 
EW