From d2ac85f7aa80185f9bbe17d7ef2a23e8b26bffef Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Mon, 13 Feb 2023 15:13:42 +0100
Subject: [PATCH] install sigproxy before start/attach

Install the signal proxy before attaching to/starting the container to
make sure there's no race-condition as revealed in the failing start/run
tests in #16901.  The tests had the valid expectation that signal
forwarding works once the container is running.

Further update the tests to account for the attach test where the
expectation is that signal forwarding works once Podman has attached to
container (or even before).

Fixes: #16901
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
Signed-off-by: Ed Santiago <santiago@redhat.com>
---
 .../infra/abi/terminal/terminal_common.go     | 14 ++++----
 test/system/032-sig-proxy.bats                | 32 +++++++++++++++----
 2 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/pkg/domain/infra/abi/terminal/terminal_common.go b/pkg/domain/infra/abi/terminal/terminal_common.go
index 3dbdfc9560..f9a012b7d2 100644
--- a/pkg/domain/infra/abi/terminal/terminal_common.go
+++ b/pkg/domain/infra/abi/terminal/terminal_common.go
@@ -84,11 +84,13 @@ func StartAttachCtr(ctx context.Context, ctr *libpod.Container, stdout, stderr,
 		streams.AttachInput = false
 	}
 
-	if !startContainer {
-		if sigProxy {
-			ProxySignals(ctr)
-		}
+	if sigProxy {
+		// To prevent a race condition, install the signal handler
+		// before starting/attaching to the container.
+		ProxySignals(ctr)
+	}
 
+	if !startContainer {
 		return ctr.Attach(streams, detachKeys, resize)
 	}
 
@@ -97,10 +99,6 @@ func StartAttachCtr(ctx context.Context, ctr *libpod.Container, stdout, stderr,
 		return err
 	}
 
-	if sigProxy {
-		ProxySignals(ctr)
-	}
-
 	if stdout == nil && stderr == nil {
 		fmt.Printf("%s\n", ctr.ID())
 	}
diff --git a/test/system/032-sig-proxy.bats b/test/system/032-sig-proxy.bats
index 422f0cae6b..9f4047e15d 100644
--- a/test/system/032-sig-proxy.bats
+++ b/test/system/032-sig-proxy.bats
@@ -3,7 +3,14 @@
 load helpers
 
 # Command to run in each of the tests.
-SLEEPLOOP='trap "echo BYE;exit 0" INT;echo READY;while :;do sleep 0.1;done'
+SLEEPLOOP='trap "echo BYE;exit 0" INT;echo READY;while :;do echo RUNNING;sleep 0.1;done'
+
+function setup() {
+    basic_setup
+
+    TESTLOG=$PODMAN_TMPDIR/container-stdout
+}
+
 
 # Main test code: wait for container to exist and be ready, send it a
 # signal, wait for container to acknowledge and exit.
@@ -26,8 +33,21 @@ function _test_sigproxy() {
           fi
     done
 
-    # Now that container exists, wait for it to declare itself READY
-    wait_for_ready $cname
+    # Now that container exists, wait for it to declare itself RUNNING
+    timeout=10
+    while :;do
+          sleep 0.5
+          if grep -q RUNNING $TESTLOG; then
+              break
+          fi
+          timeout=$((timeout - 1))
+          if [[ $timeout -eq 0 ]]; then
+              run_podman ps -a
+              echo "log from container:"
+              cat $TESTLOG
+              die "Timed out waiting for container $cname to start"
+          fi
+    done
 
     # Signal, and wait for container to exit
     kill -INT $kidpid
@@ -52,7 +72,7 @@ function _test_sigproxy() {
 
 @test "podman sigproxy test: run" {
     # We're forced to use $PODMAN because run_podman cannot be backgrounded
-    $PODMAN run -i --name c_run $IMAGE sh -c "$SLEEPLOOP" &
+    $PODMAN run -i --name c_run $IMAGE sh -c "$SLEEPLOOP" >$TESTLOG &
     local kidpid=$!
 
     _test_sigproxy c_run $kidpid
@@ -62,7 +82,7 @@ function _test_sigproxy() {
     run_podman create --name c_start $IMAGE sh -c "$SLEEPLOOP"
 
     # See above comments regarding $PODMAN and backgrounding
-    $PODMAN start --attach c_start &
+    $PODMAN start --attach c_start >$TESTLOG &
     local kidpid=$!
 
     _test_sigproxy c_start $kidpid
@@ -72,7 +92,7 @@ function _test_sigproxy() {
     run_podman run -d --name c_attach $IMAGE sh -c "$SLEEPLOOP"
 
     # See above comments regarding $PODMAN and backgrounding
-    $PODMAN attach c_attach &
+    $PODMAN attach c_attach >$TESTLOG &
     local kidpid=$!
 
     _test_sigproxy c_attach $kidpid