Closed cuu508 closed 1 year ago
hey @cuu508 have you been using this code in production? How's it behaving?
there's already a unit test for shutdown, I think it'd be useful to have a test for this scenario as well so there's no regression
Hi @goenning, I am not using it in production. I did test my changes with the one production app that I have, and it seemed to work OK, but I didn't deploy it for two reasons:
In relation to #27, here's my attempt to fix server shutdown of an idle server.
I looked at
net.http
and used the same approach. I copiedtrackListener
andcloseListeners
fromnet.http
almost as-is (added a note in README about it).The idea:
ln.Accept()
is blocking. To make it return, we close the listener.Serve()
takes a listener parameter, and the server in principle can have multiple listeners in use. Server must keep track of the listeners, so it can close them on shutdown. That's whatsrv.listeners
andtrackListener
does.This PR needs a review. I'm new to Go, and would not be aware of any subtle bugs this may be introducing.
One particular thing I'm not sure about is the listener closing function. In
net.http
, it is called "closeListenersLocked". The "Locked" part must have some significance, but I'm not sure what exactly. In general, I suspect the Close and Shutdown methods may have thread safety problems.