agent: Stop reaping child processes (resolves #1988)

The consul docker image now uses dumb-init to reap child processes, so
there's no need to reap them ourselves.

command/agent/agent.go

@@ -116,14 +116,6 @@ type Agent struct {
 	// agent methods use this, so use with care and never override
 	// outside of a unit test.
 	endpoints map[string]string
-
-	// reapLock is used to prevent child process reaping from interfering
-	// with normal waiting for subprocesses to complete. Any time you exec
-	// and wait, you should take a read lock on this mutex. Only the reaper
-	// takes the write lock. This setup prevents us from serializing all the
-	// child process management with each other, it just serializes them
-	// with the child process reaper.
-	reapLock sync.RWMutex
 }
 
 // Create is used to create a new Agent. Returns
@@ -1042,7 +1034,6 @@ func (a *Agent) AddCheck(check *structs.HealthCheck, chkType *CheckType, persist
 				Interval: chkType.Interval,
 				Timeout:  chkType.Timeout,
 				Logger:   a.logger,
-				ReapLock: &a.reapLock,
 			}
 			monitor.Start()
 			a.checkMonitors[check.CheckID] = monitor

command/agent/command.go

@@ -20,7 +20,6 @@ import (
 	"github.com/hashicorp/consul/lib"
 	"github.com/hashicorp/consul/watch"
 	"github.com/hashicorp/go-checkpoint"
-	"github.com/hashicorp/go-reap"
 	"github.com/hashicorp/go-syslog"
 	"github.com/hashicorp/logutils"
 	scada "github.com/hashicorp/scada-client/scada"
@@ -771,33 +770,6 @@ func (c *Command) Run(args []string) int {
 		defer server.Shutdown()
 	}
 
-	// Enable child process reaping
-	if (config.Reap != nil && *config.Reap) || (config.Reap == nil && os.Getpid() == 1) {
-		if !reap.IsSupported() {
-			c.Ui.Error("Child process reaping is not supported on this platform (set reap=false)")
-			return 1
-		} else {
-			logger := c.agent.logger
-			logger.Printf("[DEBUG] Automatically reaping child processes")
-
-			pids := make(reap.PidCh, 1)
-			errors := make(reap.ErrorCh, 1)
-			go func() {
-				for {
-					select {
-					case pid := <-pids:
-						logger.Printf("[DEBUG] Reaped child process %d", pid)
-					case err := <-errors:
-						logger.Printf("[ERR] Error reaping child process: %v", err)
-					case <-c.agent.shutdownCh:
-						return
-					}
-				}
-			}()
-			go reap.ReapChildren(pids, errors, c.agent.shutdownCh, &c.agent.reapLock)
-		}
-	}
-
 	// Check and shut down the SCADA listeners at the end
 	defer func() {
 		if c.scadaHttp != nil {
@@ -829,7 +801,7 @@ func (c *Command) Run(args []string) int {
 	// Register the watches
 	for _, wp := range config.WatchPlans {
 		go func(wp *watch.WatchPlan) {
-			wp.Handler = makeWatchHandler(logOutput, wp.Exempt["handler"], &c.agent.reapLock)
+			wp.Handler = makeWatchHandler(logOutput, wp.Exempt["handler"])
 			wp.LogOutput = c.logOutput
 			if err := wp.Run(httpAddr.String()); err != nil {
 				c.Ui.Error(fmt.Sprintf("Error running watch: %v", err))
@@ -1017,7 +989,7 @@ func (c *Command) handleReload(config *Config) *Config {
 	// Register the new watches
 	for _, wp := range newConf.WatchPlans {
 		go func(wp *watch.WatchPlan) {
-			wp.Handler = makeWatchHandler(c.logOutput, wp.Exempt["handler"], &c.agent.reapLock)
+			wp.Handler = makeWatchHandler(c.logOutput, wp.Exempt["handler"])
 			wp.LogOutput = c.logOutput
 			if err := wp.Run(httpAddr.String()); err != nil {
 				c.Ui.Error(fmt.Sprintf("Error running watch: %v", err))

command/agent/remote_exec.go

@@ -135,12 +135,6 @@ func (a *Agent) handleRemoteExec(msg *UserEvent) {
 		return
 	}
 
-	// Disable child process reaping so that we can get this command's
-	// return value. Note that we take the read lock here since we are
-	// waiting on a specific PID and don't need to serialize all waits.
-	a.reapLock.RLock()
-	defer a.reapLock.RUnlock()
-
 	// Ensure we write out an exit code
 	exitCode := 0
 	defer a.remoteExecWriteExitCode(&event, &exitCode)

command/agent/watch_handler.go

@@ -8,7 +8,6 @@ import (
 	"log"
 	"os"
 	"strconv"
-	"sync"
 
 	"github.com/armon/circbuf"
 	"github.com/hashicorp/consul/watch"
@@ -34,16 +33,10 @@ func verifyWatchHandler(params interface{}) error {
 }
 
 // makeWatchHandler returns a handler for the given watch
-func makeWatchHandler(logOutput io.Writer, params interface{}, reapLock *sync.RWMutex) watch.HandlerFunc {
+func makeWatchHandler(logOutput io.Writer, params interface{}) watch.HandlerFunc {
 	script := params.(string)
 	logger := log.New(logOutput, "", log.LstdFlags)
 	fn := func(idx uint64, data interface{}) {
-		// Disable child process reaping so that we can get this command's
-		// return value. Note that we take the read lock here since we are
-		// waiting on a specific PID and don't need to serialize all waits.
-		reapLock.RLock()
-		defer reapLock.RUnlock()
-
 		// Create the command
 		cmd, err := ExecScript(script)
 		if err != nil {

command/agent/watch_handler_test.go

@@ -3,7 +3,6 @@ package agent
 import (
 	"io/ioutil"
 	"os"
-	"sync"
 	"testing"
 )
 
@@ -26,7 +25,7 @@ func TestMakeWatchHandler(t *testing.T) {
 	defer os.Remove("handler_out")
 	defer os.Remove("handler_index_out")
 	script := "echo $CONSUL_INDEX >> handler_index_out && cat >> handler_out"
-	handler := makeWatchHandler(os.Stderr, script, &sync.RWMutex{})
+	handler := makeWatchHandler(os.Stderr, script)
 	handler(100, []string{"foo", "bar", "baz"})
 	raw, err := ioutil.ReadFile("handler_out")
 	if err != nil {