ruby-changes:36272
From: akr <ko1@a...>
Date: Mon, 10 Nov 2014 08:03:55 +0900 (JST)
Subject: [ruby-changes:36272] akr:r48353 (trunk): * lib/webrick/server.rb (initialize): Initialize shutdown pipe here
akr 2014-11-10 08:03:40 +0900 (Mon, 10 Nov 2014) New Revision: 48353 http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=48353 Log: * 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. Modified files: trunk/ChangeLog trunk/lib/webrick/server.rb trunk/test/webrick/test_server.rb Index: ChangeLog =================================================================== --- ChangeLog (revision 48352) +++ ChangeLog (revision 48353) @@ -1,3 +1,11 @@ https://github.com/ruby/ruby/blob/trunk/ChangeLog#L1 +Mon Nov 10 07:31:59 2014 Tanaka Akira <akr@f...> + + * 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@f...> * test/webrick/webrick.cgi: Don't use debug mode. Index: lib/webrick/server.rb =================================================================== --- lib/webrick/server.rb (revision 48352) +++ lib/webrick/server.rb (revision 48353) @@ -106,6 +106,7 @@ module WEBrick https://github.com/ruby/ruby/blob/trunk/lib/webrick/server.rb#L106 @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 https://github.com/ruby/ruby/blob/trunk/lib/webrick/server.rb#L115 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 https://github.com/ruby/ruby/blob/trunk/lib/webrick/server.rb#L159 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 https://github.com/ruby/ruby/blob/trunk/lib/webrick/server.rb#L169 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 https://github.com/ruby/ruby/blob/trunk/lib/webrick/server.rb#L196 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 https://github.com/ruby/ruby/blob/trunk/lib/webrick/server.rb#L225 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 https://github.com/ruby/ruby/blob/trunk/lib/webrick/server.rb#L317 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 Index: test/webrick/test_server.rb =================================================================== --- test/webrick/test_server.rb (revision 48352) +++ test/webrick/test_server.rb (revision 48353) @@ -34,6 +34,10 @@ class TestWEBrickServer < Test::Unit::Te https://github.com/ruby/ruby/blob/trunk/test/webrick/test_server.rb#L34 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, -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/