* lib/webrick/server.rb (module WEBrick::GenericServer): A server

  will now continue only when a StandardError subclass is raised.  For
  other exception types the error will be logged at the fatal level and
  the server will safely stop.  Based on a patch by Alex Young.
  [ruby-trunk - Feature #6236]
* test/webrick/test_server.rb:  Test for new exception handling
  behavior.  Join the server thread instead of busy-waiting for it to
  shut down to remove race conditions.


git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@35303 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

4 files changed, 102 insertions, 32 deletions

diff --git a/ChangeLog b/ChangeLog
index f552aa66fa..810c11c1e2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+Thu Apr 12 05:27:01 2012  Eric Hodel  <drbrain@segment7.net>
+
+	* lib/webrick/server.rb (module WEBrick::GenericServer):  A server
+	  will now continue only when a StandardError subclass is raised.  For
+	  other exception types the error will be logged at the fatal level and
+	  the server will safely stop.  Based on a patch by Alex Young.
+	  [ruby-trunk - Feature #6236]
+	* test/webrick/test_server.rb:  Test for new exception handling
+	  behavior.  Join the server thread instead of busy-waiting for it to
+	  shut down to remove race conditions.
+
 Thu Apr 12 03:50:44 2012  NARUSE, Yui  <naruse@ruby-lang.org>
 
 	* lib/test/unit.rb (Test::Unit:Runner::Worker#_run_suites):
diff --git a/lib/webrick/server.rb b/lib/webrick/server.rb
index c6d1e4f0f9..22b91ad535 100644
--- a/lib/webrick/server.rb
+++ b/lib/webrick/server.rb
@@ -82,6 +82,27 @@ module WEBrick
       @listeners += Utils::create_listeners(address, port, @logger)
     end
 
+    ##
+    # Starts the server and runs the +block+ for each connection.  This method
+    # does not return until the server is stopped from a signal handler or
+    # another thread using #stop or #shutdown.
+    #
+    # If the block raises a subclass of StandardError the exception is logged
+    # and ignored.  If an IOError or Errno::EBADF exception is raised the
+    # exception is ignored.  If an Exception subclass is raised the exception
+    # is logged and re-raised which stops the server.
+    #
+    # To completely shut down a server call #shutdown from ensure:
+    #
+    #   server = WEBrick::GenericServer.new
+    #   # or WEBrick::HTTPServer.new
+    #
+    #   begin
+    #     server.start
+    #   ensure
+    #     server.shutdown
+    #   end
+
     def start(&block)
       raise ServerError, "already started." if @status != :Stop
       server_type = @config[:ServerType] || SimpleServer
@@ -93,44 +114,57 @@ module WEBrick
 
         thgroup = ThreadGroup.new
         @status = :Running
-        while @status == :Running
-          begin
-            if svrs = IO.select(@listeners, nil, nil, 2.0)
-              svrs[0].each{|svr|
-                @tokens.pop          # blocks while no token is there.
-                if sock = accept_client(svr)
-                  sock.do_not_reverse_lookup = config[:DoNotReverseLookup]
-                  th = start_thread(sock, &block)
-                  th[:WEBrickThread] = true
-                  thgroup.add(th)
-                else
-                  @tokens.push(nil)
-                end
-              }
+        begin
+          while @status == :Running
+            begin
+              if svrs = IO.select(@listeners, nil, nil, 2.0)
+                svrs[0].each{|svr|
+                  @tokens.pop          # blocks while no token is there.
+                  if sock = accept_client(svr)
+                    sock.do_not_reverse_lookup = config[:DoNotReverseLookup]
+                    th = start_thread(sock, &block)
+                    th[:WEBrickThread] = true
+                    thgroup.add(th)
+                  else
+                    @tokens.push(nil)
+                  end
+                }
+              end
+            rescue Errno::EBADF, IOError => ex
+              # if the listening socket was closed in GenericServer#shutdown,
+              # IO::select raise it.
+            rescue StandardError => ex
+              msg = "#{ex.class}: #{ex.message}\n\t#{ex.backtrace[0]}"
+              @logger.error msg
+            rescue Exception => ex
+              @logger.fatal ex
+              raise
             end
-          rescue Errno::EBADF, IOError => ex
-            # if the listening socket was closed in GenericServer#shutdown,
-            # IO::select raise it.
-          rescue Exception => ex
-            msg = "#{ex.class}: #{ex.message}\n\t#{ex.backtrace[0]}"
-            @logger.error msg
           end
-        end
 
-        @logger.info "going to shutdown ..."
-        thgroup.list.each{|th| th.join if th[:WEBrickThread] }
-        call_callback(:StopCallback)
-        @logger.info "#{self.class}#start done."
-        @status = :Stop
+        ensure
+          @logger.info "going to shutdown ..."
+          thgroup.list.each{|th| th.join if th[:WEBrickThread] }
+          call_callback(:StopCallback)
+          @logger.info "#{self.class}#start done."
+          @status = :Stop
+        end
       }
     end
 
+    ##
+    # Stops the server from accepting new connections.
+
     def stop
       if @status == :Running
-        @status = :Shutdown
+        @status = :Stop
       end
     end
 
+    ##
+    # Shuts down the server and all listening sockets.  New listeners must be
+    # provided to restart the server.
+
     def shutdown
       stop
       @listeners.each{|s|
@@ -169,7 +203,7 @@ module WEBrick
         Utils::set_close_on_exec(sock)
       rescue Errno::ECONNRESET, Errno::ECONNABORTED,
              Errno::EPROTO, Errno::EINVAL => ex
-      rescue Exception => ex
+      rescue StandardError => ex
         msg = "#{ex.class}: #{ex.message}\n\t#{ex.backtrace[0]}"
         @logger.error msg
       end
diff --git a/test/webrick/test_server.rb b/test/webrick/test_server.rb
index e00449692b..534fe9b077 100644
--- a/test/webrick/test_server.rb
+++ b/test/webrick/test_server.rb
@@ -23,6 +23,32 @@ class TestWEBrickServer < Test::Unit::TestCase
     }
   end
 
+  def test_start_exception
+    stopped = 0
+    config = {
+      :StopCallback => Proc.new{ stopped += 1 },
+    }
+
+    e = assert_raises(Exception) do
+      TestWEBrick.start_server(Echo, config) { |server, addr, port, log|
+        listener = server.listeners.first
+
+        def listener.accept
+          raise Exception, 'fatal' # simulate ^C
+        end
+
+        true while server.status != :Running
+
+        TCPSocket.open(addr, port) { |sock| sock << "foo\n" }
+
+        sleep 0.1 until server.status == :Stop
+      }
+    end
+
+    assert_equal('fatal', e.message)
+    assert_equal(stopped, 1)
+  end
+
   def test_callbacks
     accepted = started = stopped = 0
     config = {
diff --git a/test/webrick/utils.rb b/test/webrick/utils.rb
index ef4654cafd..15852ca47d 100644
--- a/test/webrick/utils.rb
+++ b/test/webrick/utils.rb
@@ -36,14 +36,13 @@ module TestWEBrick
       :AccessLog => [[logger, ""]]
     }.update(config))
     begin
-      server.start
+      server_thread = server.start
       addr = server.listeners[0].addr
       block.yield([server, addr[3], addr[1], log])
     ensure
       server.shutdown
-      until server.status == :Stop
-        sleep 0.1
-      end
+
+      server_thread.join
     end
     log_string
   end