From e83756512c44294137ee3362cf131eed70663fb1 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Thu, 9 Apr 2009 23:09:03 -0700 Subject: close listeners when removing them from our array This fixes a long-standing bug where listeners would be removed from the known listener set during a reload but never correctly shut down (until reexec). Additionally, test_server was working around this bug (my fault, subconciously) as teardown did not unbind the socket, requiring the tests to grab a new port. --- lib/unicorn.rb | 6 ++++-- test/unit/test_server.rb | 15 ++++++--------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/lib/unicorn.rb b/lib/unicorn.rb index f8e0a5d..6b50319 100644 --- a/lib/unicorn.rb +++ b/lib/unicorn.rb @@ -110,8 +110,10 @@ module Unicorn @listeners.delete_if do |io| if dead_names.include?(sock_name(io)) - @io_purgatory.delete_if { |pio| pio.fileno == io.fileno } - true + @io_purgatory.delete_if do |pio| + pio.fileno == io.fileno && (pio.close rescue nil).nil? + end + (io.close rescue nil).nil? else false end diff --git a/test/unit/test_server.rb b/test/unit/test_server.rb index 3ab6b2d..742b240 100644 --- a/test/unit/test_server.rb +++ b/test/unit/test_server.rb @@ -37,7 +37,6 @@ class WebServerTest < Test::Unit::TestCase def test_preload_app_config teardown - port = unused_port tmp = Tempfile.new('test_preload_app_config') ObjectSpace.undefine_finalizer(tmp) app = lambda { || @@ -47,23 +46,22 @@ class WebServerTest < Test::Unit::TestCase lambda { |env| [ 200, { 'Content-Type' => 'text/plain' }, [ "#$$\n" ] ] } } redirect_test_io do - @server = HttpServer.new(app, :listeners => [ "127.0.0.1:#{port}"] ) + @server = HttpServer.new(app, :listeners => [ "127.0.0.1:#@port"] ) @server.start end - results = hit(["http://localhost:#{port}/"]) + results = hit(["http://localhost:#@port/"]) worker_pid = results[0].to_i tmp.sysseek(0) loader_pid = tmp.sysread(4096).to_i assert_equal worker_pid, loader_pid teardown - port = unused_port redirect_test_io do - @server = HttpServer.new(app, :listeners => [ "127.0.0.1:#{port}"], + @server = HttpServer.new(app, :listeners => [ "127.0.0.1:#@port"], :preload_app => true) @server.start end - results = hit(["http://localhost:#{port}/"]) + results = hit(["http://localhost:#@port/"]) worker_pid = results[0].to_i tmp.sysseek(0) loader_pid = tmp.sysread(4096).to_i @@ -75,16 +73,15 @@ class WebServerTest < Test::Unit::TestCase def test_broken_app teardown - port = unused_port app = lambda { |env| raise RuntimeError, "hello" } # [200, {}, []] } redirect_test_io do - @server = HttpServer.new(app, :listeners => [ "127.0.0.1:#{port}"] ) + @server = HttpServer.new(app, :listeners => [ "127.0.0.1:#@port"] ) @server.start end sock = nil assert_nothing_raised do - sock = TCPSocket.new('127.0.0.1', port) + sock = TCPSocket.new('127.0.0.1', @port) sock.syswrite("GET / HTTP/1.0\r\n\r\n") end -- cgit v1.2.3-24-ge0c7