* 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

ChangeLog

@@ -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.

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

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,