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>

2 files changed, 18 insertions, 14 deletions

diff --git a/libpod/container_api.go b/libpod/container_api.go
index df6b6e962..e1d5e15c4 100644
--- a/libpod/container_api.go
+++ b/libpod/container_api.go
@@ -328,6 +328,11 @@ func (c *Container) Exec(tty, privileged bool, env, cmd []string, user string) e
 	if err != nil {
 		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)
 	// 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
 
 	// Wait until the runtime makes the pidfile
-	// TODO: If runtime errors before the PID file is created, we have to
-	// wait for timeout here
-	if err := WaitForFile(pidFile, pidWaitTimeout*time.Millisecond); err != nil {
-		logrus.Debugf("Timed out waiting for pidfile from runtime for container %s exec", c.ID())
-
-		// 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 {
+	exited, err := WaitForFile(pidFile, chWait, pidWaitTimeout*time.Millisecond)
+	if err != nil {
+		if exited {
+			// If the runtime exited, propagate the error we got from the process.
 			return err
 		}
-
 		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
 	}
 
-	waitErr := execCmd.Wait()
+	var waitErr error
+	if !exited {
+		waitErr = <-chWait
+	}
 
 	// Lock again
 	if !c.batched {
diff --git a/libpod/util.go b/libpod/util.go
index 7007b29cd..25e78bd01 100644
--- a/libpod/util.go
+++ b/libpod/util.go
@@ -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
-func WaitForFile(path string, timeout time.Duration) error {
+func WaitForFile(path string, chWait chan error, timeout time.Duration) (bool, error) {
 	done := make(chan struct{})
 	chControl := make(chan struct{})
 	go func() {
@@ -110,11 +110,13 @@ func WaitForFile(path string, timeout time.Duration) error {
 	}()
 
 	select {
+	case e := <-chWait:
+		return true, e
 	case <-done:
-		return nil
+		return false, nil
 	case <-time.After(timeout):
 		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)
 	}
 }