Open andrewthad opened 7 months ago
Thank you for the extensive issue description.
You make some compelling observations and we'd welcome any PR adding documentation, if not addressing these issues. I've also had to dig a lot to find how to gracefully shutdown a WAI server, so I know the pain.
I don't think any of the current maintainers has headroom to work on this, but I do feel this is one of the more important issues to fix because of how core to a web server the shutdown part is.
I spent a few hours recently figuring out what the how graceful shutdown behaves. I believe that I now mostly understand what is happening, and I am writing this for these purposes:
Closing Sockets
The documentation of warp suggests closing (i.e.
Network.Socket.close
) the socket to initiate graceful shutdown. On UNIX systems, a cross-threadclose()
on a socket like this is dangerous because the kernel could hand out the same file descriptor as a result to anopen()
orsocket()
call. But with thenetwork
library, this race condition cannot happen because the implementation atomically swaps out the file descriptor with negative one (-1) before theclose()
syscall. Which means that subsequent use of the socket is not merely likely to return EBADF, it is certain to return EBADF (on Linux at least, -1 is never a valid file descriptor). To someone who is accustomed to UNIX, this improved guarantee is not obvious. It would be nice to mention in the warp docs that network's implementation ofclose()
doesn't have the dangerous race condition that UNIX sockets have. Even the documentation ofNetwork.Socket.close
itself is deficient here:This could be improved by mentioning that any calls to
send
,receive
,accept
, etc. will certainly throw exceptions after the socket is closed.A question I have: Does the user have to
close
the listening socket, or it is sufficient to callshutdown
on it instead? Warp's source code leaves me with the impression thatshutdown
should work, but I'm not certain. If calling shutdown is supported, it would be good to document this.Ending HTTP Sessions
My Specific Scenario
In the application that I am working on, a load balancer (HAProxy) sits in front of the application and runs in HTTP mode. For anyone unfamiliar with HTTP-mode load balancing, this means that the proxy opens a small number (possibly just 1) of TCP sessions to the backend and leaves these TCP sessions open forever. Any request that a client (on the frontend) makes to the proxy is handled by forwarding (basically copying) this request to one of these existing backend TCP sessions. Running a load balancer like this (instead of in TCP mode) is considered good practice and is pretty common for HTTP traffic.
Graceful Shutdown Behavior
When an HTTP server attempts to gracefully shut downs, I expect two things to happen:
accept()
)Connection: close
header in its responses and then should closing the socket (or maybe sending a TLS bye first and then closing the socket).Warp only does (1), not (2). I had incorrectly assumed that it did both. Since warp does not do both of these, then any client using a persistent connection (not just an HTTP load balancer) prevents graceful shutdown from completing. We have to use
setGracefulShutdownTimeout
and then wait until the timeout happens. This works, but it's a little unsatisfactory becauseIf warp included behavior (2) that I suggested above, then in situations where HTTP requests were coming in regularly (probably at least 1 request per second), the graceful shutdown would be even more graceful. If HTTP requests arrived very infrequently, then the server could still exhibit all three of the bad behaviors that I listed. Even so, I feel like this change this would be an improvement.
Possible improvements: