podman exec: correctly support detaching

podman exec support detaching early via the detach key sequence. In that case the podman process should exit successfully but the container exec process keeps running. Given that I could not find any existing test for the detach key functionality not even for exec I added some. This seems to reval more issues with on podman-remote. Fixes #24895 Signed-off-by: Paul Holzinger <[email protected]>
containers · Jan 22, 2025 · bd72b99 · bd72b99
1 parent 4700d15
commit bd72b99
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 6 deletions.
diff --git a/libpod/container_exec.go b/libpod/container_exec.go
@@ -291,9 +291,14 @@ func (c *Container) ExecStartAndAttach(sessionID string, streams *define.AttachS
 // ExecStartAndAttach starts and attaches to an exec session in a container.
 // newSize resizes the tty to this size before the process is started, must be nil if the exec session has no tty
 func (c *Container) execStartAndAttach(sessionID string, streams *define.AttachStreams, newSize *resize.TerminalSize, isHealthcheck bool) error {
+	unlock := true
 	if !c.batched {
 		c.lock.Lock()
-		defer c.lock.Unlock()
+		defer func() {
+			if unlock {
+				c.lock.Unlock()
+			}
+		}()
 
 		if err := c.syncContainer(); err != nil {
 			return err
@@ -348,6 +353,12 @@ func (c *Container) execStartAndAttach(sessionID string, streams *define.AttachS
 	}
 
 	tmpErr := <-attachChan
+	// user detached
+	if errors.Is(tmpErr, define.ErrDetach) {
+		// ensure we the defer does nto unlock as we are not locked here
+		unlock = false
+		return tmpErr
+	}
 	if lastErr != nil {
 		logrus.Errorf("Container %s exec session %s error: %v", c.ID(), session.ID(), lastErr)
 	}
@@ -769,11 +780,14 @@ func (c *Container) exec(config *ExecConfig, streams *define.AttachStreams, resi
 	if err != nil {
 		return -1, err
 	}
+	cleanup := true
 	defer func() {
-		if err := c.ExecRemove(sessionID, false); err != nil {
-			if retErr == nil && !errors.Is(err, define.ErrNoSuchExecSession) {
-				exitCode = -1
-				retErr = err
+		if cleanup {
+			if err := c.ExecRemove(sessionID, false); err != nil {
+				if retErr == nil && !errors.Is(err, define.ErrNoSuchExecSession) {
+					exitCode = -1
+					retErr = err
+				}
 			}
 		}
 	}()
@@ -807,6 +821,11 @@ func (c *Container) exec(config *ExecConfig, streams *define.AttachStreams, resi
 	}
 
 	if err := c.execStartAndAttach(sessionID, streams, size, isHealthcheck); err != nil {
+		// user detached, there will be no exit just exit without reporting an error
+		if errors.Is(err, define.ErrDetach) {
+			cleanup = false
+			return 0, nil
+		}
 		return -1, err
 	}
 

diff --git a/test/system/450-interactive.bats b/test/system/450-interactive.bats
@@ -39,7 +39,7 @@ function teardown() {
         kill $PODMAN_SOCAT_PID
         PODMAN_SOCAT_PID=
     fi
-    rm -f $PODMAN_TEST_PTY $PODMAN_DUMMY_PTY
+    rm -f $PODMAN_TEST_PTY $PODMAN_DUMMY
 
     basic_teardown
 }
@@ -124,4 +124,57 @@ function teardown() {
     is "$output" "$expected_tty" "passthrough-tty: tty matches"
 }
 
+@test "podman run detach keys" {
+    local cname1=c1-$(safename)
+    local cname2=c2-$(safename)
+
+    local msg=$(random_string)
+    # First "type" a command then send CTRL-p,CTRL-q sequence to the run command.
+    # We must send that sequence in two echo commands if I use a single echo it
+    # doesn't work, I don't know why.
+    # If the detach does not work podman run will hang and timeout.
+    # The sleep is needed to ensure the output can be printed before we detach.
+    (echo "echo $msg" > $PODMAN_DUMMY; sleep 1; echo -n $'\cp' > $PODMAN_DUMMY; echo -n $'\cq' > $PODMAN_DUMMY ) &
+    run_podman run -it --name $cname1 $IMAGE sh <$PODMAN_TEST_PTY
+    # Because we print to a terminal it appends "\r"
+    assert "$output" =~ "$msg"$'\r' "run output contains message"
+
+    # The container should still be running
+    run_podman inspect --format {{.State.Status}} $cname1
+    assert "$output" == "running" "container status"
+
+    # Now a second test with --detach-keys to make sure the cli option works
+    (echo "echo $msg" > $PODMAN_DUMMY; sleep 1; echo -n $'\cc' > $PODMAN_DUMMY; echo -n $'J' > $PODMAN_DUMMY ) &
+    run_podman run -it --name $cname2 --detach-keys ctrl-c,J $IMAGE sh <$PODMAN_TEST_PTY
+    assert "$output" =~ "$msg"$'\r' "run output with --detach-keys contains message"
+
+    run_podman rm -f -t0 $cname1 $cname2
+}
+
+@test "podman exec detach keys" {
+    local cname=c-$(safename)
+    run_podman run -d --name $cname $IMAGE sleep inf
+
+    local msg=$(random_string)
+    # First "type" a command then send CTRL-p,CTRL-q sequence to the exec command.
+    # If the detach does not work podman exec will hang and timeout.
+    # The sleep is needed to ensure the output can be printed before we detach.
+    (echo "echo $msg" > $PODMAN_DUMMY; sleep 1; echo -n $'\cp' > $PODMAN_DUMMY; echo -n $'\cq' > $PODMAN_DUMMY ) &
+    run_podman exec -it $cname sh <$PODMAN_TEST_PTY
+    # Because we print to a terminal it appends "\r"
+    assert "$output" =~ "$msg"$'\r' "exec output contains message"
+
+    # The previous exec session/process should still be running
+    run_podman exec $cname ps aux
+    # Match PID=2 USER=root and COMMAND=sh from the ps output
+    assert "$output" =~ "2 root.*sh" "sh exec process still running"
+
+    # Now a second test with --detach-keys to make sure the cli option works
+    (echo "echo $msg" > $PODMAN_DUMMY; sleep 1; echo -n $'\ct' > $PODMAN_DUMMY; echo -n $'f' > $PODMAN_DUMMY ) &
+    run_podman exec -it --detach-keys ctrl-t,f $cname sh <$PODMAN_TEST_PTY
+    assert "$output" =~ "$msg"$'\r' "exec output with --detach-keys contains message"
+
+    run_podman rm -f -t0 $cname
+}
+
 # vim: filetype=sh