From 2fb4810cdbeaeb7817e9d9e330db1eaae1ea5c8c Mon Sep 17 00:00:00 2001
From: Matthew Holt <Matthew.Holt+git@gmail.com>
Date: Thu, 5 Nov 2015 15:21:13 -0700
Subject: [PATCH] Fixed racy error reporting at server startup

Previously, if a listener fails to bind (for example), there was a race in caddy.go between unblocking the startup waitgroup and returning the error and putting it into errChan. Now, an error is returned directly into errChan and the closing of the startup waitgroup is defered until after that return takes place.
---
 caddy/caddy.go   | 15 ++++++---------
 server/server.go | 10 +++++-----
 2 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/caddy/caddy.go b/caddy/caddy.go
index ae01593e..839bdae0 100644
--- a/caddy/caddy.go
+++ b/caddy/caddy.go
@@ -210,16 +210,10 @@ func startServers(groupings Group) error {
 		wg.Add(1)
 		go func(s *server.Server, ln server.ListenerFile) {
 			defer wg.Done()
-
 			if ln != nil {
-				err = s.Serve(ln)
+				errChan <- s.Serve(ln)
 			} else {
-				err = s.ListenAndServe()
-			}
-
-			// "use of closed network connection" is normal if doing graceful shutdown...
-			if err != nil && !strings.Contains(err.Error(), "use of closed network connection") {
-				errChan <- err
+				errChan <- s.ListenAndServe()
 			}
 		}(s, ln)
 
@@ -240,7 +234,10 @@ func startServers(groupings Group) error {
 	// Return the first error, if any
 	select {
 	case err := <-errChan:
-		return err
+		// "use of closed network connection" is normal if it was a graceful shutdown
+		if err != nil && !strings.Contains(err.Error(), "use of closed network connection") {
+			return err
+		}
 	default:
 	}
 
diff --git a/server/server.go b/server/server.go
index c72cdd0d..d3a42076 100644
--- a/server/server.go
+++ b/server/server.go
@@ -102,7 +102,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)
+		defer close(s.startChan) // MUST defer so error is properly reported, same with all cases in this file
 		return err
 	}
 	return s.serve(ln)
@@ -112,7 +112,7 @@ func (s *Server) Serve(ln ListenerFile) error {
 func (s *Server) ListenAndServe() error {
 	err := s.setup()
 	if err != nil {
-		close(s.startChan)
+		defer close(s.startChan)
 		return err
 	}
 
@@ -130,7 +130,7 @@ func (s *Server) ListenAndServe() error {
 			}
 		}
 		if !succeeded {
-			close(s.startChan)
+			defer close(s.startChan)
 			return err
 		}
 	}
@@ -207,7 +207,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)
+			defer close(s.startChan)
 			return err
 		}
 	}
@@ -222,7 +222,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)
+		defer close(s.startChan)
 		return err
 	}