Merge pull request #5677 from giuseppe/avoid-lock-tty

exec: fix hang if control path is deleted

4 files changed, 45 insertions, 11 deletions

diff --git a/libpod/oci_attach_linux.go b/libpod/oci_attach_linux.go
index 433993edb..fb0a54bff 100644
--- a/libpod/oci_attach_linux.go
+++ b/libpod/oci_attach_linux.go
@@ -200,8 +200,10 @@ func setupStdioChannels(streams *AttachStreams, conn *net.UnixConn, detachKeys [
 		var err error
 		if streams.AttachInput {
 			_, err = utils.CopyDetachable(conn, streams.InputStream, detachKeys)
-			if connErr := conn.CloseWrite(); connErr != nil {
-				logrus.Errorf("unable to close conn: %q", connErr)
+			if err == nil {
+				if connErr := conn.CloseWrite(); connErr != nil {
+					logrus.Errorf("unable to close conn: %q", connErr)
+				}
 			}
 		}
 		stdinDone <- err
diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go
index ce888c690..6a0097b8e 100644
--- a/libpod/oci_conmon_linux.go
+++ b/libpod/oci_conmon_linux.go
@@ -575,13 +575,36 @@ func (r *ConmonOCIRuntime) HTTPAttach(ctr *Container, httpConn net.Conn, httpBuf
 	}
 }
 
+// isRetryable returns whether the error was caused by a blocked syscall or the
+// specified operation on a non blocking file descriptor wasn't ready for completion.
+func isRetryable(err error) bool {
+	if errno, isErrno := errors.Cause(err).(syscall.Errno); isErrno {
+		return errno == syscall.EINTR || errno == syscall.EAGAIN
+	}
+	return false
+}
+
+// openControlFile opens the terminal control file.
+func openControlFile(ctr *Container, parentDir string) (*os.File, error) {
+	controlPath := filepath.Join(parentDir, "ctl")
+	for i := 0; i < 600; i++ {
+		controlFile, err := os.OpenFile(controlPath, unix.O_WRONLY|unix.O_NONBLOCK, 0)
+		if err == nil {
+			return controlFile, err
+		}
+		if !isRetryable(err) {
+			return nil, errors.Wrapf(err, "could not open ctl file for terminal resize for container %s", ctr.ID())
+		}
+		time.Sleep(time.Second / 10)
+	}
+	return nil, errors.Errorf("timeout waiting for %q", controlPath)
+}
+
 // AttachResize resizes the terminal used by the given container.
 func (r *ConmonOCIRuntime) AttachResize(ctr *Container, newSize remotecommand.TerminalSize) error {
-	// TODO: probably want a dedicated function to get ctl file path?
-	controlPath := filepath.Join(ctr.bundlePath(), "ctl")
-	controlFile, err := os.OpenFile(controlPath, unix.O_WRONLY, 0)
+	controlFile, err := openControlFile(ctr, ctr.bundlePath())
 	if err != nil {
-		return errors.Wrapf(err, "could not open ctl file for terminal resize")
+		return err
 	}
 	defer controlFile.Close()
 
@@ -785,11 +808,9 @@ func (r *ConmonOCIRuntime) ExecContainer(c *Container, sessionID string, options
 
 // ExecAttachResize resizes the TTY of the given exec session.
 func (r *ConmonOCIRuntime) ExecAttachResize(ctr *Container, sessionID string, newSize remotecommand.TerminalSize) error {
-	// TODO: probably want a dedicated function to get ctl file path?
-	controlPath := filepath.Join(ctr.execBundlePath(sessionID), "ctl")
-	controlFile, err := os.OpenFile(controlPath, unix.O_WRONLY, 0)
+	controlFile, err := openControlFile(ctr, ctr.execBundlePath(sessionID))
 	if err != nil {
-		return errors.Wrapf(err, "could not open ctl file for terminal resize for container %s exec session %s", ctr.ID(), sessionID)
+		return err
 	}
 	defer controlFile.Close()
 
diff --git a/test/e2e/exec_test.go b/test/e2e/exec_test.go
index ab806f683..5d0d6e689 100644
--- a/test/e2e/exec_test.go
+++ b/test/e2e/exec_test.go
@@ -122,6 +122,18 @@ var _ = Describe("Podman exec", func() {
 		Expect(session.ExitCode()).To(Equal(100))
 	})
 
+	It("podman exec terminal doesn't hang", func() {
+		setup := podmanTest.Podman([]string{"run", "-dti", fedoraMinimal, "sleep", "+Inf"})
+		setup.WaitWithDefaultTimeout()
+		Expect(setup.ExitCode()).To(Equal(0))
+
+		for i := 0; i < 5; i++ {
+			session := podmanTest.Podman([]string{"exec", "-lti", "true"})
+			session.WaitWithDefaultTimeout()
+			Expect(session.ExitCode()).To(Equal(0))
+		}
+	})
+
 	It("podman exec pseudo-terminal sanity check", func() {
 		setup := podmanTest.Podman([]string{"run", "--detach", "--name", "test1", fedoraMinimal, "sleep", "+Inf"})
 		setup.WaitWithDefaultTimeout()
diff --git a/utils/utils.go b/utils/utils.go
index 3c8c0a9b0..cf58ca3fb 100644
--- a/utils/utils.go
+++ b/utils/utils.go
@@ -65,7 +65,6 @@ func CopyDetachable(dst io.Writer, src io.Reader, keys []byte) (written int64, e
 					break
 				}
 				if i == len(keys)-1 {
-					// src.Close()
 					return 0, ErrDetach
 				}
 				nr, er = src.Read(buf)