diff options
author | akr <akr@b2dd03c8-39d4-4d8f-98ff-823fe69b080e> | 2014-11-09 23:03:40 +0000 |
---|---|---|
committer | akr <akr@b2dd03c8-39d4-4d8f-98ff-823fe69b080e> | 2014-11-09 23:03:40 +0000 |
commit | 2a9ea113550c8358788dd1a3f163aba963b28d96 (patch) | |
tree | d26226f60d07337043b0a21af221d458f3eb51a3 | |
parent | ef82fcf0674611b4c373dc8497da19d5bc92466a (diff) | |
download | ruby-2a9ea113550c8358788dd1a3f163aba963b28d96.tar.gz |
* lib/webrick/server.rb (initialize): Initialize shutdown pipe here
to avoid race condition.
(cleanup_shutdown_pipe): New private method.
(cleanup_listener): Extracted from shutdown method.
Call this method from start method to avoid race condition.
git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@48353 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
-rw-r--r-- | ChangeLog | 8 | ||||
-rw-r--r-- | lib/webrick/server.rb | 79 | ||||
-rw-r--r-- | test/webrick/test_server.rb | 4 |
3 files changed, 51 insertions, 40 deletions
@@ -1,3 +1,11 @@ +Mon Nov 10 07:31:59 2014 Tanaka Akira <akr@fsij.org> + + * lib/webrick/server.rb (initialize): Initialize shutdown pipe here + to avoid race condition. + (cleanup_shutdown_pipe): New private method. + (cleanup_listener): Extracted from shutdown method. + Call this method from start method to avoid race condition. + Mon Nov 10 05:57:53 2014 Tanaka Akira <akr@fsij.org> * test/webrick/webrick.cgi: Don't use debug mode. diff --git a/lib/webrick/server.rb b/lib/webrick/server.rb index d173c22906..dd1be6ab23 100644 --- a/lib/webrick/server.rb +++ b/lib/webrick/server.rb @@ -106,6 +106,7 @@ module WEBrick @logger.info("ruby #{rubyv}") @listeners = [] + @shutdown_pipe_r = @shutdown_pipe_w = nil unless @config[:DoNotListen] if @config[:Listen] warn(":Listen option is deprecated; use GenericServer#listen") @@ -114,8 +115,8 @@ module WEBrick if @config[:Port] == 0 @config[:Port] = @listeners[0].addr[1] end + @shutdown_pipe_r, @shutdown_pipe_w = IO.pipe end - @shutdown_pipe_w = nil end ## @@ -158,9 +159,6 @@ module WEBrick raise ServerError, "already started." if @status != :Stop server_type = @config[:ServerType] || SimpleServer - shutdown_pipe_r, shutdown_pipe_w = IO.pipe - @shutdown_pipe_w = shutdown_pipe_w - server_type.start{ @logger.info \ "#{self.class}#start: pid=#{$$} port=#{@config[:Port]}" @@ -171,8 +169,8 @@ module WEBrick begin while @status == :Running begin - if svrs = IO.select([shutdown_pipe_r, *@listeners], nil, nil, 2.0) - if svrs[0].include? shutdown_pipe_r + if svrs = IO.select([@shutdown_pipe_r, *@listeners], nil, nil, 2.0) + if svrs[0].include? @shutdown_pipe_r break end svrs[0].each{|svr| @@ -198,16 +196,9 @@ module WEBrick raise end end - ensure - shutdown_pipe_r.close - if !shutdown_pipe_w.closed? - begin - shutdown_pipe_w.close - rescue IOError # Another thread closed shutdown_pipe_w. - end - end - @shutdown_pipe_w = nil + cleanup_shutdown_pipe + cleanup_listener @status = :Shutdown @logger.info "going to shutdown ..." thgroup.list.each{|th| th.join if th[:WEBrickThread] } @@ -234,34 +225,14 @@ module WEBrick def shutdown stop - shutdown_pipe_w = @shutdown_pipe_w - @shutdown_pipe_w = nil - if shutdown_pipe_w && !shutdown_pipe_w.closed? + shutdown_pipe_w = @shutdown_pipe_w # another thread may modify @shutdown_pipe_w. + if shutdown_pipe_w begin - shutdown_pipe_w.close - rescue IOError # Another thread closed shutdown_pipe_w. + shutdown_pipe_w.write_nonblock "a" + rescue IO::WaitWritable + rescue IOError # closed by another thread. end end - - @listeners.each{|s| - if @logger.debug? - addr = s.addr - @logger.debug("close TCPSocket(#{addr[2]}, #{addr[1]})") - end - begin - s.shutdown - rescue Errno::ENOTCONN - # when `Errno::ENOTCONN: Socket is not connected' on some platforms, - # call #close instead of #shutdown. - # (ignore @config[:ShutdownSocketWithoutClose]) - s.close - else - unless @config[:ShutdownSocketWithoutClose] - s.close - end - end - } - @listeners.clear end ## @@ -346,5 +317,33 @@ module WEBrick cb.call(*args) end end + + def cleanup_shutdown_pipe + @shutdown_pipe_r.close + @shutdown_pipe_w.close + @shutdown_pipe_r = @shutdown_pipe_w = nil + end + + def cleanup_listener + @listeners.each{|s| + if @logger.debug? + addr = s.addr + @logger.debug("close TCPSocket(#{addr[2]}, #{addr[1]})") + end + begin + s.shutdown + rescue Errno::ENOTCONN + # when `Errno::ENOTCONN: Socket is not connected' on some platforms, + # call #close instead of #shutdown. + # (ignore @config[:ShutdownSocketWithoutClose]) + s.close + else + unless @config[:ShutdownSocketWithoutClose] + s.close + end + end + } + @listeners.clear + end end # end of GenericServer end diff --git a/test/webrick/test_server.rb b/test/webrick/test_server.rb index fa7fc940ca..f765da6861 100644 --- a/test/webrick/test_server.rb +++ b/test/webrick/test_server.rb @@ -34,6 +34,10 @@ class TestWEBrickServer < Test::Unit::TestCase def listener.to_io # IO.select invokes #to_io. raise SignalException, 'SIGTERM' # simulate signal in main thread end + def listener.shutdown + end + def listener.close + end server = WEBrick::HTTPServer.new({ :BindAddress => "127.0.0.1", :Port => 0, |