From f3f2456b0451d93a539eac63dac93a748e01317f Mon Sep 17 00:00:00 2001 From: Michael Crosby Date: Mon, 13 Jan 2014 16:10:23 -0800 Subject: [PATCH] Simplify chroot wait, address code review issues Docker-DCO-1.1-Signed-off-by: Michael Crosby (github: crosbymichael) --- container.go | 6 ++++-- execdriver/chroot/driver.go | 22 +++++----------------- execdriver/driver.go | 5 ++++- execdriver/lxc/driver.go | 7 ++++--- 4 files changed, 17 insertions(+), 23 deletions(-) diff --git a/container.go b/container.go index 4e66539c5c..e8277131cf 100644 --- a/container.go +++ b/container.go @@ -708,7 +708,7 @@ func (container *Container) Start() (err error) { } waitLock := make(chan struct{}) - f := func(process *execdriver.Process) { + callback := func(process *execdriver.Process) { container.State.SetRunning(process.Pid()) if process.Tty { // The callback is called after the process Start() @@ -724,7 +724,9 @@ func (container *Container) Start() (err error) { close(waitLock) } - go container.monitor(f) + // We use a callback here instead of a goroutine and an chan for + // syncronization purposes + go container.monitor(callback) // Start should not return until the process is actually running <-waitLock diff --git a/execdriver/chroot/driver.go b/execdriver/chroot/driver.go index eb2525be6d..e5dc69131c 100644 --- a/execdriver/chroot/driver.go +++ b/execdriver/chroot/driver.go @@ -9,11 +9,11 @@ import ( type driver struct { } -func NewDriver() (execdriver.Driver, error) { +func NewDriver() (*driver, error) { return &driver{}, nil } -func (d *driver) String() string { +func (d *driver) Name() string { return "chroot" } @@ -23,7 +23,7 @@ func (d *driver) Run(c *execdriver.Process, startCallback execdriver.StartCallba c.Rootfs, "/.dockerinit", "-driver", - d.String(), + d.Name(), } params = append(params, c.Entrypoint) params = append(params, c.Arguments...) @@ -43,24 +43,12 @@ func (d *driver) Run(c *execdriver.Process, startCallback execdriver.StartCallba return -1, err } - var ( - waitErr error - waitLock = make(chan struct{}) - ) - go func() { - if err := c.Wait(); err != nil { - waitErr = err - } - close(waitLock) - }() - if startCallback != nil { startCallback(c) } - <-waitLock - - return c.GetExitCode(), waitErr + err = c.Wait() + return c.GetExitCode(), err } func (d *driver) Kill(p *execdriver.Process, sig int) error { diff --git a/execdriver/driver.go b/execdriver/driver.go index cb9df96e79..fac4e27704 100644 --- a/execdriver/driver.go +++ b/execdriver/driver.go @@ -14,7 +14,7 @@ type Driver interface { // TODO: @crosbymichael @creack wait should probably return the exit code Wait(id string, duration time.Duration) error // Wait on an out of process...process - lxc ghosts Version() string - String() string + Name() string } // Network settings of the container @@ -43,6 +43,9 @@ type Process struct { } func (c *Process) Pid() int { + if c.Process == nil { + return -1 + } return c.Process.Pid } diff --git a/execdriver/lxc/driver.go b/execdriver/lxc/driver.go index b220ff146e..e37e105b5b 100644 --- a/execdriver/lxc/driver.go +++ b/execdriver/lxc/driver.go @@ -29,7 +29,7 @@ type driver struct { sharedRoot bool } -func NewDriver(root string, apparmor bool) (execdriver.Driver, error) { +func NewDriver(root string, apparmor bool) (*driver, error) { // setup unconfined symlink if err := linkLxcStart(root); err != nil { return nil, err @@ -41,7 +41,7 @@ func NewDriver(root string, apparmor bool) (execdriver.Driver, error) { }, nil } -func (d *driver) String() string { +func (d *driver) Name() string { return "lxc" } @@ -53,7 +53,7 @@ func (d *driver) Run(c *execdriver.Process, startCallback execdriver.StartCallba "--", c.InitPath, "-driver", - d.String(), + d.Name(), } if c.Network != nil { @@ -195,6 +195,7 @@ func (d *driver) waitForStart(c *execdriver.Process, waitLock chan struct{}) err select { case <-waitLock: // If the process dies while waiting for it, just return + return nil if c.ProcessState != nil && c.ProcessState.Exited() { return nil }