From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-2.9 required=3.0 tests=ALL_TRUSTED,BAYES_00 shortcircuit=no autolearn=unavailable version=3.3.2 X-Original-To: yahns-public@yhbt.net Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id DB32C1F5AA; Mon, 9 Mar 2015 04:46:43 +0000 (UTC) Date: Mon, 9 Mar 2015 04:46:43 +0000 From: Eric Wong To: yahns-public@yhbt.net Subject: [PATCH] acceptor: close inherited-but-unneeded sockets Message-ID: <20150309044643.GA13968@dcvr.yhbt.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline List-Id: 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