From 991a4f1f05ffb66434e1b7dc94345803dbbfcb03 Mon Sep 17 00:00:00 2001 From: Naitik Shah Date: Mon, 25 Mar 2013 13:33:05 -0700 Subject: [PATCH] fix bug introduced with systemd compatibility --- grace.go | 41 ++++++++++++++++++++---------- gracehttp/http_test.go | 26 +++++++++++++++++++ gracehttp/testserver/testserver.go | 11 ++++++-- 3 files changed, 63 insertions(+), 15 deletions(-) diff --git a/grace.go b/grace.go index 89cc2cc..cf5b703 100644 --- a/grace.go +++ b/grace.go @@ -40,13 +40,23 @@ const ( type Listener interface { net.Listener + // Will indicate that a Close is requested preventing further Accept. It will + // also wait for the active connections to be terminated before returning. + CloseRequest() + // Will return the underlying file representing this Listener. File() (f *os.File, err error) } +// A realListener is a real file backed net.Listener. +type realListener interface { + net.Listener + File() (f *os.File, err error) +} + // A goroutine based counter that provides graceful Close for listeners. type listener struct { - Listener + realListener closed bool // Indicates we're already closed. closeRequest chan bool // Send a bool here to indicate we want to Close. allClosed chan bool // Receive from here will indicate a clean Close. @@ -65,9 +75,9 @@ func (c conn) Close() error { } // Wraps an existing File listener to provide a graceful Close() process. -func NewListener(l Listener) Listener { +func NewListener(l realListener) Listener { i := &listener{ - Listener: l, + realListener: l, closeRequest: make(chan bool), allClosed: make(chan bool), counter: make(chan bool), @@ -97,20 +107,24 @@ func (l *listener) enabler() { } } -func (l *listener) Close() error { +func (l *listener) CloseRequest() { if l.closed == true { - return nil + return } l.closeRequest <- true <-l.allClosed - return l.Listener.Close() +} + +func (l *listener) Close() error { + l.CloseRequest() + return l.realListener.Close() } func (l *listener) Accept() (net.Conn, error) { if l.closed == true { return nil, ErrAlreadyClosed } - c, err := l.Listener.Accept() + c, err := l.realListener.Accept() if err != nil { if strings.HasSuffix(err.Error(), errClosed) { return nil, ErrAlreadyClosed @@ -132,16 +146,17 @@ func Wait(listeners []Listener) (err error) { sig := <-ch switch sig { case syscall.SIGTERM: - if os.Getppid() == 1 { // init provided sockets dont close - return nil - } var wg sync.WaitGroup wg.Add(len(listeners)) for _, l := range listeners { go func(l Listener) { - cErr := l.Close() - if cErr != nil { - err = cErr + if os.Getppid() == 1 { // init provided sockets dont actually close + l.CloseRequest() + } else { + cErr := l.Close() + if cErr != nil { + err = cErr + } } wg.Done() }(l) diff --git a/gracehttp/http_test.go b/gracehttp/http_test.go index 9a7631e..fdb6c8f 100644 --- a/gracehttp/http_test.go +++ b/gracehttp/http_test.go @@ -24,8 +24,17 @@ const ( // The amount of time for the long HTTP request. This should be // bigger than the value above. slowHttpWait = time.Second * 4 + + // Debug logging. + debugLog = false ) +func debug(format string, a ...interface{}) { + if debugLog { + println(fmt.Sprintf(format, a...)) + } +} + // The response from the test server. type response struct { Sleep time.Duration @@ -162,6 +171,7 @@ func (h *harness) RemoveExe() { // Helper for sending a single request. func (h *harness) SendOne(duration time.Duration, addr string, pid int) { + debug("Send One pid=%d duration=%s", pid, duration) client := &http.Client{ Transport: &http.Transport{DisableKeepAlives: true}, } @@ -179,6 +189,7 @@ func (h *harness) SendOne(duration time.Duration, addr string, pid int) { if pid != res.Pid { h.T.Fatalf("Didn't get expected pid %d instead got %d", pid, res.Pid) } + debug("Request Done pid=%d duration=%s", pid, duration) h.RequestWaitGroup.Done() } @@ -186,6 +197,7 @@ func (h *harness) SendOne(duration time.Duration, addr string, pid int) { func (h *harness) SendRequest() { pid := h.MostRecentProcess().Pid for _, addr := range h.Addr { + debug("Added 2 Requests") h.RequestWaitGroup.Add(2) go h.SendOne(time.Second*0, addr, pid) go h.SendOne(slowHttpWait, addr, pid) @@ -199,21 +211,35 @@ func (h *harness) Wait() { // The main test case. func TestComplex(t *testing.T) { + debug("Started TestComplex") h := &harness{ ImportPath: "github.com/daaku/go.grace/gracehttp/testserver", T: t, } + debug("Building") h.Build() + debug("Initial Start") h.Start() + debug("Send Request 1") h.SendRequest() + debug("Sleeping 1") time.Sleep(processWait) + debug("Restart 1") h.Restart() + debug("Send Request 2") h.SendRequest() + debug("Sleeping 2") time.Sleep(processWait) + debug("Restart 2") h.Restart() + debug("Send Request 3") h.SendRequest() + debug("Sleeping 3") time.Sleep(processWait) + debug("Stopping") h.Stop() + debug("Waiting") h.Wait() + debug("Removing Executable") h.RemoveExe() } diff --git a/gracehttp/testserver/testserver.go b/gracehttp/testserver/testserver.go index df834db..820dfff 100644 --- a/gracehttp/testserver/testserver.go +++ b/gracehttp/testserver/testserver.go @@ -24,15 +24,22 @@ var ( func main() { flag.Parse() - err := json.NewEncoder(os.Stderr).Encode(&response{Pid: os.Getpid()}) + err := flag.Set("gracehttp.log", "false") + if err != nil { + log.Fatalf("Error setting gracehttp.log: %s", err) + } + err = json.NewEncoder(os.Stderr).Encode(&response{Pid: os.Getpid()}) if err != nil { log.Fatalf("Error writing startup json: %s", err) } - gracehttp.Serve( + err = gracehttp.Serve( gracehttp.Handler{*address0, newHandler()}, gracehttp.Handler{*address1, newHandler()}, gracehttp.Handler{*address2, newHandler()}, ) + if err != nil { + log.Fatalf("Error in gracehttp.Serve: %s", err) + } } func newHandler() http.Handler {