braintree / manners

A polite Go HTTP server that shuts down gracefully.

Manners

A polite webserver for Go.

Manners allows you to shut your Go webserver down gracefully, without dropping any requests. It can act as a drop-in replacement for the standard library's http.ListenAndServe function:

func main() {
  handler := MyHTTPHandler()
  manners.ListenAndServe(":7000", handler)
}

Then, when you want to shut the server down:

manners.Close()

(Note that this does not block until all the requests are finished. Rather, the call to manners.ListenAndServe will stop blocking when all the requests are finished.)

Manners ensures that all requests are served by incrementing a WaitGroup when a request comes in and decrementing it when the request finishes.

If your request handler spawns Goroutines that are not guaranteed to finish with the request, you can ensure they are also completed with the StartRoutine and FinishRoutine functions on the server.

Known Issues

Manners does not correctly shut down long-lived keepalive connections when issued a shutdown command. Clients on an idle keepalive connection may see a connection reset error rather than a close. See #13 for details.

Compatability

Manners 0.3.0 and above uses standard library functionality introduced in Go 1.3.

Installation

go get github.com/braintree/manners

manners's Issues

Update README to warn about keep-alive and graceful shutdown

After #10 was applied (which I agree with) I'm seeing behavior that confused me for a bit, and may confuse others.

manners relies on the socket open/close events to increment/decrement the WaitGroup counter. That makes sense.

If you use a reverse proxy, and that proxy uses keep-alive to reduce latency to the upstream, then this will cause manners to block until those keep-alive sockets are closed.

I noticed this when I was attempting a graceful shutdown of my Go process, and my signal handler logged the shutdown request normally, but the process never exited.

In my case I don't need keep-alive enabled on the reverse proxy, and disabling keep-alive fixed my shutdown issues. Others may run into this, so it may be worth a warning in the README.

Thanks, -- James

How about websockets?

Hi! I just wanted to know how manners get along with websocket connections...

ListenAndServeTLS and letsencrypt

Hi,
is there a way for you to provide a working sample on how to user manners with letsencrypt ?
I actually use this command :
log.Fatal(manners.ListenAndServe(ApiIP, Router))

I would love to use an autoupdate process by crontab like with nginx or apache ...

Thanks for your help !
Stéphane

manners doesn't set http.Request.TLS

When using manners.NewWithServer, r.TLS is nil

Has global state, can only have one graceful server

The use of global state means it isn't possible to start multiple servers in one process neatly. You might want to do this to serve http and https from the same process for instance, or two independent pieces of code can't be composed if they both use manners correctly.

Could we instead lose the "implicitly return 200" and say in the instructions that if you want to do asynchronous computation you should use a waitgroup yourself. This would be more idiomatic, I think, rather than hiding what is really happening. It would also obviate the need for global StartRoutine/FinishRoutine methods.

You could also have a GracefulHTTPHandler which requires a .Wait() method, which could be another way to do it (then the user could just embed a waitgroup on their handler type).

race condition in tests

It seems to me that line 40

<-done

should be moved to the end of the function to avoid a race condition between:

• the server shutting down (and assignment exited=true) and
• the check if exited {...

I'm aware that this test runs only on one thread and the handler takes quite long to complete (so the race is highly unlikely), but I believe that the Go memory model makes this change mandatory (to get correct, predictable results according to the language semantics).

Another race is in the TestShutdown. Concurrent execution may lead to a situation where the server is still running but the other goroutine went ahead and successfully connected.

race condition in listener.go

GracefulListener is subject to race condition on its open bool field. According to the documentation of standard library

Multiple goroutines may invoke methods on a Listener simultaneously.

thus contending for read/write access to the open field. It seems that GracefulListener needs some kind of synchronization mechanism, e.g., a mutex.

Signal example should use syscall.SIGTERM instead of os.Kill

Regarding the documented signals example, os.Kill can't be caught, it's just there for killing other processes.

os.Kill is like kill -9 which kills a process immediately

syscall.SIGTERM is equivalent to kill which allows the process time to cleanup

signal.Notify(sigchan, os.Interrupt, syscall.SIGTERM)

ref: #9

TestShutdown intermittently fails

Go 1.4.2 - non-b... branch

--- FAIL: TestShutdown (0.00s)
    server_test.go:64: Waitcount should be one, got 0

Serving + Closing in 2 separate routines is racy

Running ListenAndServe() and Close() in 2 separate routines causes the race detector to pick up a race condition, based on the fact that defaultServer has no synchronization protection around it.

Fixed by #37

Worth noting, this change would make Close() block until ListenAndServe is actually called

License

The project has no LICENSE file nor mention of any license that I could find in the source. Could you please clarify?

[retracted]

[resolved]

doesn't shutdown when used with clients using persistent connections

Try the program below. If you run it and press Ctrl-C it exits. If you run it, then hit localhost:8080/foo with firefox or chrome, then Ctrl-C the program, it will hang. This happens because the http.Server created inside func (s *GracefulServer) Serve(...) doesn't have a read timeout set.

There's an easy fix, I'll send a pull request.

package main

import (
    "fmt"
    "github.com/braintree/manners"
    "io"
    "net/http"
    "os"
    "os/signal"
    "syscall"
)

var quitChan = make(chan os.Signal, 1)
var server = manners.NewServer()

func Hello(w http.ResponseWriter, req *http.Request) {
    io.WriteString(w, "hello, world!\n")
}

/* it looks like manners only shuts down if the client doesn't use persistent connection;
   e.g. if you run this, then make a request with curl, then ctrl-c this process,
   it shuts down gracefully. But if you make the request with Chrome or Firefox,
   then ctrl-c this process, the http server doesn't quit until you close the
   browser tab (firefox) or entire browser (chrome) */
func waitForSignal() {
    fmt.Println("waiting for signal")
    <-quitChan
    fmt.Println("got signal")
    server.Shutdown <- true
}

func main() {

    signal.Notify(quitChan, syscall.SIGINT, syscall.SIGKILL, syscall.SIGHUP, syscall.SIGTERM)
    http.HandleFunc("/foo", Hello)
    go waitForSignal()
    fmt.Println("listening...")
    server.ListenAndServe(":8080", nil)
    fmt.Println("exiting")
}

Fix ListenAndServe(addr, nil)

The main example from the package comment doesn't work:

    http.Handle("/hello", func(w http.ResponseWriter, r *http.Request) {
      w.Write([]byte("Hello\n"))
    })

    log.Fatal(manners.ListenAndServe(":8080", nil))

This gives the panic mentioned in #39.

(See also #40, but IMHO it would be better to make this work than to show a workaround in the README.)

Panic...

So was giving this library a try, but I can't seem to get it working. Wrote a tiny sample app just to make sure there was nothing special in my code.

package main

import (
    "log"
    "net/http"

    "github.com/braintree/manners"
)

func main() {
    http.Handle("/hello", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        w.Write([]byte("Hello\n"))
    }))
    log.Fatal(manners.ListenAndServe(":8080", nil))
}

This panics and returns the following stack trace.

2016/05/19 16:49:55 http: panic serving [::1]:39150: runtime error: invalid memory address or nil pointer dereference
goroutine 18 [running]:
net/http.(*conn).serve.func1(0xc8200ce000)
    /usr/lib/go/src/net/http/server.go:1389 +0xc1
panic(0x711720, 0xc82000a230)
    /usr/lib/go/src/runtime/panic.go:443 +0x4e9
github.com/braintree/manners.(*gracefulHandler).ServeHTTP(0xc8200babe0, 0x7f381b775130, 0xc8200e2000, 0xc8200de000)
    /home/quest/go/src/github.com/braintree/manners/server.go:277 +0x78
net/http.serverHandler.ServeHTTP(0xc82007a080, 0x7f381b775130, 0xc8200e2000, 0xc8200de000)
    /usr/lib/go/src/net/http/server.go:2081 +0x19e
net/http.(*conn).serve(0xc8200ce000)
    /usr/lib/go/src/net/http/server.go:1472 +0xf2e
created by net/http.(*Server).Serve
    /usr/lib/go/src/net/http/server.go:2137 +0x44e
2016/05/19 16:49:57 http: panic serving [::1]:39156: runtime error: invalid memory address or nil pointer dereference

Am I doing something wrong? The docs don't wrap the Handle passed function with http.HandlerFunc, but that is required for it to compile. Has this been tested against Go 1.6?

A blocking version of Close()

Hello, I recently discovered this package and found out that its Close() method doesn't block. I have a helper package github.com/xlab/closer that calls functions upon app's exit, so as soon as all the "cleanup" functions are done it closes the app with os.Exit().

I've tried to use that package with manners and the result is quite ugly:

func main() {
    // start the http server
    server := manners.NewWithServer(&http.Server{Addr: ":8080", Handler: nil})

    // a channel to check on exit that ListenAndServe is done
    exitChan := make(chan struct{})
    closer.Bind(func() {
        <-exitChan
    })
    // bind server.Close to be called if app is exiting because of a signal
    closer.Bind(func() {
        server.Close()
    })
    if err := server.ListenAndServe(); err != nil {
        log.Fatalln(err)
    }
    close(exitChan)
}

As you can see, I've been forced to add an additional channel that will ensure blocking for the graceful shutdown circuit. Instead of this (only 1 additional line of code) doing the same thing:

func main() {
    server := manners.NewWithServer(&http.Server{Addr: ":8080", Handler: nil})
    closer.Bind(func() { server.BlockingClose() }) // that's it!
    server.ListenAndServe()
}

Adopted practice

Many services that offer graceful shutdown feature either provide a blocking close method or a channel one could listen to. Example of channel: nsq.Consumer

type Consumer struct {

    // read from this channel to block until consumer is cleanly stopped
    StopChan chan int
    // contains filtered or unexported fields
}

// Stop will initiate a graceful stop of the Consumer (permanent)
//
// NOTE: receive on StopChan to block until this process completes
func (r *Consumer) Stop()

Allow /debug endpoints be access while service shutting down

Sometimes if the service is left hanging waiting for shutdown you want to access the /debug endpoints provided by some packages, such as the one you get by importing /net/http/pprof

Any idea on how to handle such a scenario? Ideally I wouldnt want to assign two port to all my go services.

How the channel would get the shutdown message?

Hi,

Could you clarify for me - which way I should send the message to the server. Shutdown ?

My real goal (why I'm actually here) is to know how can I build the web server, which I can restart in the same way as apache, for example.

Like service apache2 restart

Using your concept, I think it's possible to do the "stop" operation, at least.
Could you clarify how can I, from the external environment, put the message to the server.Shutdown channel?

Thanks

Multiple calls to Close will panic

It's probably worth noting in the documentation that, for the non-backwards-compat branch, more than one call to Close will panic.

Can you please allow GracefulListener to be constructed outside of manners package?

I was using the old manners, which allowed custom built listeners to be passed to the HTTP serving func.

The new API /almost/ allows you to do that, except I can't create a GracefulListener struct from outside of manners because it has private fields.

This would be solved if you either make those public, or if you can add NewListener func:

// this...
type GracefulListener struct {
    net.Listener
    Open bool
    Server *GracefulServer
}

// or this...
func NewLisneter(listener net.Listener, s *GracefulServer) {
    return &GracefulListener { listener, true, s }
}

Please merge non-backwards-compat

I've been reviewing the non-backwards-compat branch, and it looks like the perfect implementation of a no-timeout graceful-stopping http.Server. Is there anything stopping it from becoming master?

Thanks for the work!

1.0.0 Release?

Any chance of getting a 1.0.0 release where the API won't change?