exec: don't wait for pidfile when the runtime exited

don't wait for the timeout to expire if the runtime process exited.
I've noticed podman to hang on exit and keeping the container lock
taken when the OCI runtime already exited.

Additionally, it reduces the waiting time as we won't hit the 25
milliseconds waiting time in the worst case.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
This commit is contained in:
Giuseppe Scrivano
2018-11-20 11:06:20 +01:00
parent 049defa984
commit 070ce0c855
2 changed files with 18 additions and 14 deletions

View File

@ -328,6 +328,11 @@ func (c *Container) Exec(tty, privileged bool, env, cmd []string, user string) e
if err != nil { if err != nil {
return errors.Wrapf(err, "error exec %s", c.ID()) return errors.Wrapf(err, "error exec %s", c.ID())
} }
chWait := make(chan error)
go func() {
chWait <- execCmd.Wait()
}()
defer close(chWait)
pidFile := c.execPidPath(sessionID) pidFile := c.execPidPath(sessionID)
// 60 second seems a reasonable time to wait // 60 second seems a reasonable time to wait
@ -336,18 +341,12 @@ func (c *Container) Exec(tty, privileged bool, env, cmd []string, user string) e
const pidWaitTimeout = 60000 const pidWaitTimeout = 60000
// Wait until the runtime makes the pidfile // Wait until the runtime makes the pidfile
// TODO: If runtime errors before the PID file is created, we have to exited, err := WaitForFile(pidFile, chWait, pidWaitTimeout*time.Millisecond)
// wait for timeout here if err != nil {
if err := WaitForFile(pidFile, pidWaitTimeout*time.Millisecond); err != nil { if exited {
logrus.Debugf("Timed out waiting for pidfile from runtime for container %s exec", c.ID()) // If the runtime exited, propagate the error we got from the process.
// Check if an error occurred in the process before we made a pidfile
// TODO: Wait() here is a poor choice - is there a way to see if
// a process has finished, instead of waiting for it to finish?
if err := execCmd.Wait(); err != nil {
return err return err
} }
return errors.Wrapf(err, "timed out waiting for runtime to create pidfile for exec session in container %s", c.ID()) return errors.Wrapf(err, "timed out waiting for runtime to create pidfile for exec session in container %s", c.ID())
} }
@ -389,7 +388,10 @@ func (c *Container) Exec(tty, privileged bool, env, cmd []string, user string) e
locked = false locked = false
} }
waitErr := execCmd.Wait() var waitErr error
if !exited {
waitErr = <-chWait
}
// Lock again // Lock again
if !c.batched { if !c.batched {

View File

@ -90,7 +90,7 @@ func MountExists(specMounts []spec.Mount, dest string) bool {
} }
// WaitForFile waits until a file has been created or the given timeout has occurred // WaitForFile waits until a file has been created or the given timeout has occurred
func WaitForFile(path string, timeout time.Duration) error { func WaitForFile(path string, chWait chan error, timeout time.Duration) (bool, error) {
done := make(chan struct{}) done := make(chan struct{})
chControl := make(chan struct{}) chControl := make(chan struct{})
go func() { go func() {
@ -110,11 +110,13 @@ func WaitForFile(path string, timeout time.Duration) error {
}() }()
select { select {
case e := <-chWait:
return true, e
case <-done: case <-done:
return nil return false, nil
case <-time.After(timeout): case <-time.After(timeout):
close(chControl) close(chControl)
return errors.Wrapf(ErrInternal, "timed out waiting for file %s", path) return false, errors.Wrapf(ErrInternal, "timed out waiting for file %s", path)
} }
} }