From 88c646c86c911999592af5c242b4c91f8c9b5001 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Fri, 30 Oct 2015 00:19:43 -0600 Subject: [PATCH] core: Start() blocks until servers finish starting Also improved/clarified some docs --- caddy/caddy.go | 35 ++++++++++++++++++++++++++++------- caddy/caddy_test.go | 32 ++++++++++++++++++++++++++++++++ caddy/restart.go | 2 +- server/server.go | 30 +++++++++++++++++++++++++----- 4 files changed, 86 insertions(+), 13 deletions(-) create mode 100644 caddy/caddy_test.go diff --git a/caddy/caddy.go b/caddy/caddy.go index 578b894b2..aa492b06b 100644 --- a/caddy/caddy.go +++ b/caddy/caddy.go @@ -11,6 +11,10 @@ // // You should use caddy.Wait() to wait for all Caddy servers // to quit before your process exits. +// +// Importing this package has the side-effect of trapping +// SIGINT on all platforms and SIGUSR1 on not-Windows systems. +// It has to do this in order to perform shutdowns or reloads. package caddy import ( @@ -83,11 +87,13 @@ const ( // Start starts Caddy with the given Caddyfile. If cdyfile // is nil or the process is forked from a parent as part of // a graceful restart, Caddy will check to see if Caddyfile -// was piped from stdin and use that. +// was piped from stdin and use that. It blocks until all the +// servers are listening. // // If this process is a fork and no Caddyfile was piped in, -// an error will be returned. If this process is NOT a fork -// and cdyfile is nil, a default configuration will be assumed. +// an error will be returned (the Restart() function does this +// for you automatically). If this process is NOT a fork and +// cdyfile is nil, a default configuration will be assumed. // In any case, an error is returned if Caddy could not be // started. func Start(cdyfile Input) error { @@ -175,9 +181,12 @@ func Start(cdyfile Input) error { // startServers starts all the servers in groupings, // taking into account whether or not this process is -// a child from a graceful restart or not. +// a child from a graceful restart or not. It blocks +// until the servers are listening. func startServers(groupings Group) error { - for i, group := range groupings { + var startupWg sync.WaitGroup + + for _, group := range groupings { s, err := server.New(group.BindAddr.String(), group.Configs) if err != nil { log.Fatal(err) @@ -206,8 +215,9 @@ func startServers(groupings Group) error { } wg.Add(1) - go func(s *server.Server, i int, ln server.ListenerFile) { + go func(s *server.Server, ln server.ListenerFile) { defer wg.Done() + if ln != nil { err = s.Serve(ln) } else { @@ -222,16 +232,27 @@ func startServers(groupings Group) error { log.Println(err) } } - }(s, i, ln) + }(s, ln) + + startupWg.Add(1) + go func(s *server.Server) { + defer startupWg.Done() + s.WaitUntilStarted() + }(s) serversMu.Lock() servers = append(servers, s) serversMu.Unlock() } + + startupWg.Wait() + return nil } // Stop stops all servers. It blocks until they are all stopped. +// It does NOT execute shutdown callbacks that may have been +// configured by middleware (they are executed on SIGINT). func Stop() error { letsencrypt.Deactivate() diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go new file mode 100644 index 000000000..ae84b31df --- /dev/null +++ b/caddy/caddy_test.go @@ -0,0 +1,32 @@ +package caddy + +import ( + "net/http" + "testing" + "time" +) + +func TestCaddyStartStop(t *testing.T) { + caddyfile := "localhost:1984\ntls off" + + for i := 0; i < 2; i++ { + err := Start(CaddyfileInput{Contents: []byte(caddyfile)}) + if err != nil { + t.Fatalf("Error starting, iteration %d: %v", i, err) + } + + client := http.Client{ + Timeout: time.Duration(2 * time.Second), + } + resp, err := client.Get("http://localhost:1984") + if err != nil { + t.Fatalf("Expected GET request to succeed (iteration %d), but it failed: %v", i, err) + } + resp.Body.Close() + + err = Stop() + if err != nil { + t.Fatalf("Error stopping, iteration %d: %v", i, err) + } + } +} diff --git a/caddy/restart.go b/caddy/restart.go index 7921f3754..eae8604ff 100644 --- a/caddy/restart.go +++ b/caddy/restart.go @@ -93,7 +93,7 @@ func Restart(newCaddyfile Input) error { sigwpipe.Close() // close our copy of the write end of the pipe or we might be stuck answer, err := ioutil.ReadAll(sigrpipe) if err != nil || len(answer) == 0 { - log.Println("restart: child failed to answer; changes not applied") + log.Println("restart: child failed to initialize; changes not applied") return incompleteRestartErr } diff --git a/server/server.go b/server/server.go index 9e7bcb389..a1ab6c58b 100644 --- a/server/server.go +++ b/server/server.go @@ -32,6 +32,7 @@ type Server struct { listener ListenerFile // the listener which is bound to the socket listenerMu sync.Mutex // protects listener httpWg sync.WaitGroup // used to wait on outstanding connections + startChan chan struct{} // used to block until server is finished starting } type ListenerFile interface { @@ -42,6 +43,11 @@ type ListenerFile interface { // New creates a new Server which will bind to addr and serve // the sites/hosts configured in configs. This function does // not start serving. +// +// Do not re-use a server (start, stop, then start again). We +// could probably add more locking to make this possible, but +// as it stands, you should dispose of a server after stopping it. +// The behavior of serving with a spent server is undefined. func New(addr string, configs []Config) (*Server, error) { var tls bool if len(configs) > 0 { @@ -56,8 +62,9 @@ func New(addr string, configs []Config) (*Server, error) { // WriteTimeout: 2 * time.Minute, // MaxHeaderBytes: 1 << 16, }, - tls: tls, - vhosts: make(map[string]virtualHost), + tls: tls, + vhosts: make(map[string]virtualHost), + startChan: make(chan struct{}), } s.Handler = s // this is weird, but whatever @@ -94,6 +101,7 @@ func New(addr string, configs []Config) (*Server, error) { func (s *Server) Serve(ln ListenerFile) error { err := s.setup() if err != nil { + close(s.startChan) return err } return s.serve(ln) @@ -103,11 +111,13 @@ func (s *Server) Serve(ln ListenerFile) error { func (s *Server) ListenAndServe() error { err := s.setup() if err != nil { + close(s.startChan) return err } ln, err := net.Listen("tcp", s.Addr) if err != nil { + close(s.startChan) return err } @@ -136,6 +146,7 @@ func (s *Server) serve(ln ListenerFile) error { return serveTLSWithSNI(s, s.listener, tlsConfigs) } + close(s.startChan) // unblock anyone waiting for this to start listening return s.Server.Serve(s.listener) } @@ -182,6 +193,7 @@ func serveTLSWithSNI(s *Server, ln net.Listener, tlsConfigs []TLSConfig) error { config.Certificates[i], err = tls.LoadX509KeyPair(tlsConfig.Certificate, tlsConfig.Key) config.Certificates[i].OCSPStaple = tlsConfig.OCSPStaple if err != nil { + close(s.startChan) return err } } @@ -196,6 +208,7 @@ func serveTLSWithSNI(s *Server, ln net.Listener, tlsConfigs []TLSConfig) error { // TLS client authentication, if user enabled it err = setupClientAuth(tlsConfigs, config) if err != nil { + close(s.startChan) return err } @@ -205,7 +218,7 @@ func serveTLSWithSNI(s *Server, ln net.Listener, tlsConfigs []TLSConfig) error { // on POSIX systems. ln = tls.NewListener(ln, config) - // Begin serving; block until done + close(s.startChan) // unblock anyone waiting for this to start listening return s.Server.Serve(ln) } @@ -215,7 +228,7 @@ func serveTLSWithSNI(s *Server, ln net.Listener, tlsConfigs []TLSConfig) error { // seconds); on Windows it will close the listener // immediately. func (s *Server) Stop() error { - s.Server.SetKeepAlivesEnabled(false) // TODO: Does this even do anything? :P + s.Server.SetKeepAlivesEnabled(false) if runtime.GOOS != "windows" { // force connections to close after timeout @@ -229,7 +242,7 @@ func (s *Server) Stop() error { // Wait for remaining connections to finish or // force them all to close after timeout select { - case <-time.After(5 * time.Second): // TODO: make configurable? + case <-time.After(5 * time.Second): // TODO: make configurable case <-done: } } @@ -246,6 +259,13 @@ func (s *Server) Stop() error { return err } +// WaitUntilStarted blocks until the server s is started, meaning +// that practically the next instruction is to start the server loop. +// It also unblocks if the server encounters an error during startup. +func (s *Server) WaitUntilStarted() { + <-s.startChan +} + // ListenerFd gets the file descriptor of the listener. func (s *Server) ListenerFd() uintptr { s.listenerMu.Lock()