Merge pull request #873 from mholt/fix-restart

Fix restart on USR1 not loading new Caddyfile

caddy.go

@@ -65,7 +65,7 @@ type Instance struct {
 	caddyfileInput Input
 
 	// wg is used to wait for all servers to shut down
-	wg sync.WaitGroup
+	wg *sync.WaitGroup
 
 	// servers is the list of servers with their listeners...
 	servers []serverListener
@@ -89,12 +89,14 @@ func (i *Instance) Stop() error {
 	}
 
 	// splice instance list to delete this one
+	instancesMu.Lock()
 	for j, other := range instances {
 		if other == i {
 			instances = append(instances[:j], instances[j+1:]...)
 			break
 		}
 	}
+	instancesMu.Unlock()
 
 	return nil
 }
@@ -119,6 +121,9 @@ func (i *Instance) shutdownCallbacks() []error {
 func (i *Instance) Restart(newCaddyfile Input) (*Instance, error) {
 	log.Println("[INFO] Reloading")
 
+	i.wg.Add(1)
+	defer i.wg.Done()
+
 	// run restart callbacks
 	for _, fn := range i.onRestart {
 		err := fn()
@@ -142,7 +147,7 @@ func (i *Instance) Restart(newCaddyfile Input) (*Instance, error) {
 	}
 
 	// create new instance; if the restart fails, it is simply discarded
-	newInst := &Instance{serverType: newCaddyfile.ServerType()}
+	newInst := &Instance{serverType: newCaddyfile.ServerType(), wg: i.wg}
 
 	// attempt to start new instance
 	err := startWithListenerFds(newCaddyfile, newInst, restartFds)
@@ -150,15 +155,8 @@ func (i *Instance) Restart(newCaddyfile Input) (*Instance, error) {
 		return i, err
 	}
 
-	// success! bump the old instance out so it will be garbage-collected
-	instancesMu.Lock()
-	for j, other := range instances {
-		if other == i {
-			instances = append(instances[:j], instances[j+1:]...)
-			break
-		}
-	}
-	instancesMu.Unlock()
+	// success! stop the old instance
+	i.Stop()
 
 	log.Println("[INFO] Reloading complete")
 
@@ -365,7 +363,7 @@ func (i *Instance) Caddyfile() Input {
 // This function blocks until all the servers are listening.
 func Start(cdyfile Input) (*Instance, error) {
 	writePidFile()
-	inst := &Instance{serverType: cdyfile.ServerType()}
+	inst := &Instance{serverType: cdyfile.ServerType(), wg: new(sync.WaitGroup)}
 	return inst, startWithListenerFds(cdyfile, inst, nil)
 }
 
@@ -405,13 +403,10 @@ func startWithListenerFds(cdyfile Input, inst *Instance, restartFds map[string]r
 		return err
 	}
 
-	if restartFds == nil {
-		// run startup callbacks since this is not a restart
-		for _, startupFunc := range inst.onStartup {
-			err := startupFunc()
-			if err != nil {
-				return err
-			}
+	for _, startupFunc := range inst.onStartup {
+		err := startupFunc()
+		if err != nil {
+			return err
 		}
 	}
 
@@ -532,7 +527,6 @@ func startServers(serverList []Server, inst *Instance, restartFds map[string]res
 					return err
 				}
 				file.Close()
-				delete(restartFds, addr)
 			}
 		}
 
@@ -552,15 +546,6 @@ func startServers(serverList []Server, inst *Instance, restartFds map[string]res
 		inst.servers = append(inst.servers, serverListener{server: s, listener: ln})
 	}
 
-	// Close the remaining (unused) file descriptors to free up resources
-	// and stop old servers that aren't used anymore
-	for key, old := range restartFds {
-		if err := old.server.Stop(); err != nil {
-			log.Printf("[ERROR] Stopping %s: %v", old.server.Address(), err)
-		}
-		delete(restartFds, key)
-	}
-
 	// Log errors that may be returned from Serve() calls,
 	// these errors should only be occurring in the server loop.
 	go func() {

caddyhttp/httpserver/plugin.go

@@ -51,7 +51,8 @@ var contexts []*httpContext
 
 func newContext() caddy.Context {
 	context := &httpContext{keysToSiteConfigs: make(map[string]*SiteConfig)}
-	contexts = append(contexts, context)
+	// put the new context at start to allow setup of directives on new instance
+	contexts = append([]*httpContext{context}, contexts...)
 	return context
 }