stream.jl: accept re: Shutting down an HttpServer gracefully

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

stream.jl: accept re: Shutting down an HttpServer gracefully

Eric Forgy
Hello,

I've been talking to myself over at an issue I submitted yesterday to HttpServer.jl
You can tell I am a total newbie to server stuff, but it was a good learning experience working through all the code and traced the issue down to

handle_http_request

where a call to 

accept

is made. accept takes an open socket (or creates one) and waits for a client to connect. When a client connects, it returns an active socket.

To shut down a running HttpServer, an obvious thing to try I think is 

close(server.http.sock)

This does shutdown the server, but it does not die gracefully because accept from handle_http_request is still waiting for a client to connect.

Here is the code:

function accept(server::LibuvServer, client::LibuvStream)
   
if server.status != StatusActive
       
throw(ArgumentError("server not connected, make sure \"listen\" has been called"))
   
end
   
while isopen(server)
        err
= accept_nonblock(server,client)
       
if err == 0
           
return client
        elseif err
!= UV_EAGAIN
            uv_error
("accept",err)
       
end
        stream_wait
(server,server.connectnotify)
   
end
   
uv_error("accept", UV_ECONNABORTED)
end

I traced the history of stream.jl (there must be an easier way than what I did - binary search clicking through "Blame" :)) to this commit:
  • https://github.com/JuliaLang/julia/commit/860501f669cba02789f1aa936e0665b3f4d8a629
where the following error was added:

error("accept: Server was closed while attempting to accept a client.")

Since then, I've written a new method:

Base.accept(server::Server,client=TCPSocket())

and added it to my fork of HttpServer.jl and am considering submitting a PR. It basically allows the loop to exit gracefully, returns an inactive socket and deals with the fallout in handle_http_request from receiving a dud socket.

HOWEVER, I think an easier way to allow us to gracefully shutdown an HttpServer is to simply remove that error message.

Why should it be an error if a server is closed while attempting to accept a client?

If there is a good reason, then I can submit my PR to HttpServer.jl, but just wondering if removing that uv_error message would be acceptable.

Thanks for your patience and consideration :)
Reply | Threaded
Open this post in threaded view
|

Re: stream.jl: accept re: Shutting down an HttpServer gracefully

Eric Forgy
PS: I imagine anyone seeing this would likely have seen my PR here too, but Keno made a suggestion so I'll try to submit a PR to HttpServer.jl instead. Thanks.