search

braintree / manners

A polite Go HTTP server that shuts down gracefully.
MIT License
997 stars 103 forks source link

Add call to wg.Wait() before returning from Serve(), fix server tests #10

Closed kainosnoema closed 10 years ago

kainosnoema commented 10 years ago

Though the readme states clearly that "the call to server.ListenAndServe will stop blocking when all the requests are finished", this doesn't seem to be the case. I've put together multiple tests, and at least in Go 1.2, as soon as the listener is closed and Accept() starts returning an error, http.Serve returns with that error immediately. Is it possible this is new behavior in Go 1.2?

In any case, manners isn't calling wg.Wait() anywhere, so the process exits and all in-flight routines are killed. Essentially, it's currently doing nothing, as far as I can tell. The tests in server_test.go pass because the process is still running when the test completes, meaning the goroutines eventually finish even after the server has shut down, even though in most environments they would be killed.

This patch adds a call to wg.Wait() before returning from Serve() when the listener has been closed, and fixes the test to actually check whether the request has completed before the server shuts down. Tested and working well with Go 1.2.

lionelbarrow commented 10 years ago

This was a pretty egregious error on my part. I introduced this bug in this commit 4 months ago and basically broke the library. I apologize. Thanks a lot for catching this.