core: Start() blocks until servers finish starting

Also improved/clarified some docs

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()
 

caddy/caddy_test.go

+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)
+		}
+	}
+}

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
 	}
 

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()