Browse Source

Merge pull request #14476 from crosbymichael/execid-growth-fix

Prevent uncontrolled exec config growth
Jessie Frazelle 10 years ago
parent
commit
382799a642
4 changed files with 43 additions and 8 deletions
  1. 3 3
      daemon/container.go
  2. 1 0
      daemon/daemon.go
  3. 39 4
      daemon/exec.go
  4. 0 1
      daemon/inspect.go

+ 3 - 3
daemon/container.go

@@ -835,13 +835,11 @@ func (container *Container) monitorExec(execConfig *execConfig, callback execdri
 		err      error
 		exitCode int
 	)
-
 	pipes := execdriver.NewPipes(execConfig.StreamConfig.stdin, execConfig.StreamConfig.stdout, execConfig.StreamConfig.stderr, execConfig.OpenStdin)
 	exitCode, err = container.daemon.Exec(container, execConfig, pipes, callback)
 	if err != nil {
 		logrus.Errorf("Error running command in existing container %s: %s", container.ID, err)
 	}
-
 	logrus.Debugf("Exec task in container %s exited with code %d", container.ID, exitCode)
 	if execConfig.OpenStdin {
 		if err := execConfig.StreamConfig.stdin.Close(); err != nil {
@@ -859,7 +857,9 @@ func (container *Container) monitorExec(execConfig *execConfig, callback execdri
 			logrus.Errorf("Error closing terminal while running in container %s: %s", container.ID, err)
 		}
 	}
-
+	// remove the exec command from the container's store only and not the
+	// daemon's store so that the exec command can be inspected.
+	container.execCommands.Delete(execConfig.ID)
 	return err
 }
 

+ 1 - 0
daemon/daemon.go

@@ -735,6 +735,7 @@ func NewDaemon(config *Config, registryService *registry.Service) (daemon *Daemo
 	d.RegistryService = registryService
 	d.EventsService = eventsService
 	d.root = config.Root
+	go d.execCommandGC()
 
 	if err := d.restore(); err != nil {
 		return nil, err

+ 39 - 4
daemon/exec.go

@@ -6,6 +6,7 @@ import (
 	"io/ioutil"
 	"strings"
 	"sync"
+	"time"
 
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/daemon/execdriver"
@@ -21,12 +22,13 @@ type execConfig struct {
 	ID            string
 	Running       bool
 	ExitCode      int
-	ProcessConfig execdriver.ProcessConfig
+	ProcessConfig *execdriver.ProcessConfig
 	StreamConfig
 	OpenStdin  bool
 	OpenStderr bool
 	OpenStdout bool
 	Container  *Container
+	canRemove  bool
 }
 
 type execStore struct {
@@ -128,7 +130,7 @@ func (d *Daemon) ContainerExecCreate(config *runconfig.ExecConfig) (string, erro
 		user = container.Config.User
 	}
 
-	processConfig := execdriver.ProcessConfig{
+	processConfig := &execdriver.ProcessConfig{
 		Tty:        config.Tty,
 		Entrypoint: entrypoint,
 		Arguments:  args,
@@ -221,7 +223,6 @@ func (d *Daemon) ContainerExecStart(execName string, stdin io.ReadCloser, stdout
 			execErr <- fmt.Errorf("Cannot run exec command %s in container %s: %s", execName, container.ID, err)
 		}
 	}()
-
 	select {
 	case err := <-attachErr:
 		if err != nil {
@@ -236,7 +237,7 @@ func (d *Daemon) ContainerExecStart(execName string, stdin io.ReadCloser, stdout
 }
 
 func (d *Daemon) Exec(c *Container, execConfig *execConfig, pipes *execdriver.Pipes, startCallback execdriver.StartCallback) (int, error) {
-	exitStatus, err := d.execDriver.Exec(c.command, &execConfig.ProcessConfig, pipes, startCallback)
+	exitStatus, err := d.execDriver.Exec(c.command, execConfig.ProcessConfig, pipes, startCallback)
 
 	// On err, make sure we don't leave ExitCode at zero
 	if err != nil && exitStatus == 0 {
@@ -248,3 +249,37 @@ func (d *Daemon) Exec(c *Container, execConfig *execConfig, pipes *execdriver.Pi
 
 	return exitStatus, err
 }
+
+// execCommandGC runs a ticker to clean up the daemon references
+// of exec configs that are no longer part of the container.
+func (d *Daemon) execCommandGC() {
+	for range time.Tick(5 * time.Minute) {
+		var (
+			cleaned          int
+			liveExecCommands = d.containerExecIds()
+		)
+		for id, config := range d.execCommands.s {
+			if config.canRemove {
+				cleaned++
+				d.execCommands.Delete(id)
+			} else {
+				if _, exists := liveExecCommands[id]; !exists {
+					config.canRemove = true
+				}
+			}
+		}
+		logrus.Debugf("clean %d unused exec commands", cleaned)
+	}
+}
+
+// containerExecIds returns a list of all the current exec ids that are in use
+// and running inside a container.
+func (d *Daemon) containerExecIds() map[string]struct{} {
+	ids := map[string]struct{}{}
+	for _, c := range d.containers.List() {
+		for _, id := range c.execCommands.List() {
+			ids[id] = struct{}{}
+		}
+	}
+	return ids
+}

+ 0 - 1
daemon/inspect.go

@@ -124,6 +124,5 @@ func (daemon *Daemon) ContainerExecInspect(id string) (*execConfig, error) {
 	if err != nil {
 		return nil, err
 	}
-
 	return eConfig, nil
 }