Make the no-wait-for-splay behaviour only for shutdown

We probably _do_ want to wait for the splay when the reload signal is
received, just not the kill signal.

child/child.go

@@ -197,7 +197,7 @@ func (c *Child) Reload() error {
 		c.Lock()
 		defer c.Unlock()
 
-		c.kill()
+		c.kill(false)
 		return c.start()
 	}
 
@@ -223,14 +223,25 @@ func (c *Child) Kill() {
 	log.Printf("[INFO] (child) killing process")
 	c.Lock()
 	defer c.Unlock()
-	c.kill()
+	c.kill(false)
 }
 
 // Stop behaves almost identical to Kill except it suppresses future processes
 // from being started by this child and it prevents the killing of the child
 // process from sending its value back up the exit channel. This is useful
 // when doing a graceful shutdown of an application.
 func (c *Child) Stop() {
+	c.internalStop(false)
+}
+
+// StopImmediately behaves almost identical to Stop except it does not wait
+// for any random splay if configured. This is used for performing a fast
+// shutdown of consul-template and its children when a kill signal is received.
+func (c *Child) StopImmediately() {
+	c.internalStop(true)
+}
+
+func (c *Child) internalStop(immediately bool) {
 	log.Printf("[INFO] (child) stopping process")
 
 	c.Lock()
@@ -242,9 +253,9 @@ func (c *Child) Stop() {
 		log.Printf("[WARN] (child) already stopped")
 		return
 	}
-	c.stopped = true
-	c.kill()
+	c.kill(immediately)
 	close(c.stopCh)
+	c.stopped = true
 }
 
 func (c *Child) start() error {
@@ -354,18 +365,18 @@ func (c *Child) reload() error {
 	return c.signal(c.reloadSignal)
 }
 
-func (c *Child) kill() {
+func (c *Child) kill(immediately bool) {
 	if !c.running() {
 		return
 	}
 
 	exited := false
 	process := c.cmd.Process
 
-	if !c.running() {
-		log.Printf("[DEBUG] (runner) Kill() called but process dead; not waiting for splay.")
-	} else if c.stopped {
-		log.Printf("[DEBUG] (runner) Stopping; not waiting for splay.")
+	if c.cmd.ProcessState != nil {
+		log.Printf("[DEBUG] (child) Kill() called but process dead; not waiting for splay.")
+	} else if immediately {
+		log.Printf("[DEBUG] (child) Kill() called but performing immediate shutdown; not waiting for splay.")
 	} else {
 		select {
 		case <-c.stopCh:

child/child_test.go

@@ -400,7 +400,7 @@ func TestStop_noWaitForSplay(t *testing.T) {
 	c := testChild(t)
 	c.command = "bash"
 	c.args = []string{"-c", "trap 'echo one; exit' SIGUSR1; while true; do sleep 0.2; done"}
-	c.splay = 10 * time.Second
+	c.splay = 100 * time.Second
 	c.reloadSignal = nil
 	c.killSignal = syscall.SIGUSR1
 
@@ -415,7 +415,7 @@ func TestStop_noWaitForSplay(t *testing.T) {
 	time.Sleep(fileWaitSleepDelay)
 
 	killStartTime := time.Now()
-	c.Stop()
+	c.StopImmediately()
 	killEndTime := time.Now()
 
 	expected := "one\n"
@@ -427,3 +427,31 @@ func TestStop_noWaitForSplay(t *testing.T) {
 		t.Error("expected not to wait for splay")
 	}
 }
+
+func TestStop_childAlreadyDead(t *testing.T) {
+	t.Parallel()
+	c := testChild(t)
+	c.command = "bash"
+	c.args = []string{"-c", "exit 1"}
+	c.splay = 100 * time.Second
+	c.reloadSignal = nil
+	c.killSignal = syscall.SIGTERM
+
+	out := gatedio.NewByteBuffer()
+	c.stdout, c.stderr = out, out
+
+	if err := c.Start(); err != nil {
+		t.Fatal(err)
+	}
+
+	// For some reason bash doesn't start immediately
+	time.Sleep(fileWaitSleepDelay)
+
+	killStartTime := time.Now()
+	c.Stop()
+	killEndTime := time.Now()
+
+	if killEndTime.Sub(killStartTime) > 500*time.Millisecond {
+		t.Error("expected not to wait for splay")
+	}
+}

cli.go

@@ -152,7 +152,7 @@ func (cli *CLI) Run(args []string) int {
 				go runner.Start()
 			case *config.KillSignal:
 				fmt.Fprintf(cli.errStream, "Cleaning up...\n")
-				runner.Stop()
+				runner.StopImmediately()
 				return ExitCodeInterrupt
 			case signals.SignalLookup["SIGCHLD"]:
 				// The SIGCHLD signal is sent to the parent of a child process when it

manager/runner.go

@@ -384,26 +384,13 @@ func (r *Runner) Start() {
 
 // Stop halts the execution of this runner and its subprocesses.
 func (r *Runner) Stop() {
-	r.stopLock.Lock()
-	defer r.stopLock.Unlock()
-
-	if r.stopped {
-		return
-	}
-
-	log.Printf("[INFO] (runner) stopping")
-	r.stopDedup()
-	r.stopWatcher()
-	r.stopChild()
-
-	if err := r.deletePid(); err != nil {
-		log.Printf("[WARN] (runner) could not remove pid at %q: %s",
-			r.config.PidFile, err)
-	}
-
-	r.stopped = true
+	r.internalStop(false)
+}
 
-	close(r.DoneCh)
+// StopImmediately behaves like Stop but won't wait for any splay on any child
+// process it may be running.
+func (r *Runner) StopImmediately() {
+	r.internalStop(true)
 }
 
 // TemplateRenderedCh returns a channel that will be triggered when one or more
@@ -431,6 +418,29 @@ func (r *Runner) RenderEvents() map[string]*RenderEvent {
 	return times
 }
 
+func (r *Runner) internalStop(immediately bool) {
+	r.stopLock.Lock()
+	defer r.stopLock.Unlock()
+
+	if r.stopped {
+		return
+	}
+
+	log.Printf("[INFO] (runner) stopping")
+	r.stopDedup()
+	r.stopWatcher()
+	r.stopChild(immediately)
+
+	if err := r.deletePid(); err != nil {
+		log.Printf("[WARN] (runner) could not remove pid at %q: %s",
+			r.config.PidFile, err)
+	}
+
+	r.stopped = true
+
+	close(r.DoneCh)
+}
+
 func (r *Runner) stopDedup() {
 	if r.dedup != nil {
 		log.Printf("[DEBUG] (runner) stopping de-duplication manager")
@@ -445,13 +455,18 @@ func (r *Runner) stopWatcher() {
 	}
 }
 
-func (r *Runner) stopChild() {
+func (r *Runner) stopChild(immediately bool) {
 	r.childLock.RLock()
 	defer r.childLock.RUnlock()
 
 	if r.child != nil {
-		log.Printf("[DEBUG] (runner) stopping child process")
-		r.child.Stop()
+		if immediately {
+			log.Printf("[DEBUG] (runner) stopping child process immediately")
+			r.child.StopImmediately()
+		} else {
+			log.Printf("[DEBUG] (runner) stopping child process")
+			r.child.Stop()
+		}
 	}
 }