core: Fix bug that caused parent process to block indefinitely

The error channel used when starting all the servers must be buffered so that, even if there are no errors at startup, the returns that insert into the error channel will not be blocked, since after startup, nobody is reading that channel anymore.

2 files changed, 19 insertions, 11 deletions

diff --git a/caddy/caddy.go b/caddy/caddy.go
index 5c6f75764..9988476d3 100644
--- a/caddy/caddy.go
+++ b/caddy/caddy.go
@@ -94,10 +94,10 @@ const (
 // 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 {
-	// TODO: What if already started -- is that an error?
+func Start(cdyfile Input) (err error) {
+	defer func() { signalParent(err == nil) }()
 
-	var err error
+	// TODO: What if already started -- is that an error?
 
 	// Input must never be nil; try to load something
 	if cdyfile == nil {
@@ -161,13 +161,6 @@ func Start(cdyfile Input) error {
 		}
 	}
 
-	// Tell parent process that we got this
-	if IsRestart() {
-		ppipe := os.NewFile(3, "") // parent is listening on pipe at index 3
-		ppipe.Write([]byte("success"))
-		ppipe.Close()
-	}
-
 	return nil
 }
 
@@ -177,7 +170,7 @@ func Start(cdyfile Input) error {
 // until the servers are listening.
 func startServers(groupings bindingGroup) error {
 	var startupWg sync.WaitGroup
-	errChan := make(chan error)
+	errChan := make(chan error, len(groupings)) // must be buffered to allow Serve functions below to return if stopped later
 
 	for _, group := range groupings {
 		s, err := server.New(group.BindAddr.String(), group.Configs)
diff --git a/caddy/helpers.go b/caddy/helpers.go
index c7cc479ce..8189c760b 100644
--- a/caddy/helpers.go
+++ b/caddy/helpers.go
@@ -38,6 +38,21 @@ func checkFdlimit() {
 	}
 }
 
+// signalParent tells the parent our status using pipe at index 3.
+// If this process is not a restart, this function does nothing.
+// Calling this is vital so that the parent process can unblock and
+// either continue running or kill itself.
+func signalParent(success bool) {
+	if IsRestart() {
+		ppipe := os.NewFile(3, "") // parent is listening on pipe at index 3
+		if success {
+			// Tell parent process that we're OK so it can quit now
+			ppipe.Write([]byte("success"))
+		}
+		ppipe.Close()
+	}
+}
+
 // caddyfileGob maps bind address to index of the file descriptor
 // in the Files array passed to the child process. It also contains
 // the caddyfile contents. Used only during graceful restarts.