[ruby-core:44332] Re: [ruby-trunk - Feature #6236][Closed] WEBrick::HTTPServer swallows Exception

On 13/04/12 09:01, naruse (Yui NARUSE) wrote:
>
> Issue #6236 has been updated by naruse (Yui NARUSE).
>
> Status changed from Assigned to Closed
>
> This ticket looks intended to allow ^C shutting down the server.
> So current change to shutdown other than StandardError is too wide, it should be only Interrupt.

You've reverted to catching non-StandardError Exceptions.  Do you 
disagree with my argument here?

> I couldn't see that it was reasonable for WEBrick to expect to handle any of the Exception subclasses that aren't StandardErrors.  This problem is caused precisely because WEBrick tried to handle something it shouldn't have, and I'd expect a similar problem with other Exceptions.
>
> For this immediate problem, yes, `rescue Interrupt` would work just as well.  I think it would be masking a wider issue.

If so, why?

-- 
Alex



>
> drbrain (Eric Hodel) wrote:
>> I misunderstood the intended use of start/stop/shutdown, so I think your patch should be applied.
>>
>> I assumed you could start the server again after stopping it like:
>>
>>    start ->  stop ->  start ->  stop ->  shutdown
>>
>> since stop does not close the listening sockets, so having the :Shutdown state made no sense.
>>
>> I changed :Shutdown to :Stop in GenericServer#stop because my test would hang waiting for the server to reach :Stop state after stop was called which would not bring the server to the :Stop state (due to a swallowed Exception).  I changed TESTWEBrick::start_server to use Thread.join instead of a busy-loop this will no longer be a problem.
>
> Using Thread.join itself is not wrong, but on such case the change will break existing code.
> Anyway I fixed it in r35315.
> ----------------------------------------
> Feature #6236: WEBrick::HTTPServer swallows Exception
> https://bugs.ruby-lang.org/issues/6236#change-25875
>
> Author: regularfry (Alex Young)
> Status: Closed
> Priority: Normal
> Assignee: nahi (Hiroshi Nakamura)
> Category: lib
> Target version: 2.0.0
>
>
> At the moment when using WEBrick you've always got to remember to define a signal handler to be able to kill the server when you're done with it.  This is annoying and makes it more painful to use than it should be, because if you realise you've forgotten to define a trap("INT") handler after you've started the server, all you can do is kill -9 the process.  This also catches out people learning the library more than it should.  It shouldn't be the web server's job to take over process management, but that's what it ends up doing.
>
> The reason this happens is because webrick/server.rb uses `rescue Exception` around its accept loop.  This is more broad than it should be.  The attached patch replaces this with a `rescue StandardError`, and causes other Exception subclasses to be logged and re-raised.  This makes WEBrick::HTTPServer somewhat more friendly to use at the command-line.
>
> If you Ctrl-c out of a `server.start` loop with this patch applied, you can't restart the server because it leaves internal state lying around, but I think it's still an improvement over the current situation.
>
>