From 953437b10276e77a6f907bc60bb0f4ecc4c8e3fa Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Fri, 29 May 2026 21:42:25 -0700 Subject: [PATCH] executor: avoid runc stdin shutdown hangs Forward non-tty stdin through an os.Pipe so runc receives an *os.File instead of the caller's reader. This lets runc exit after the container process is killed without waiting on Go's internal stdin copy. Add gateway coverage for graceful pid1 exit, release-based cleanup, and explicit SIGKILL while pid1 stdin is still open. Signed-off-by: Tonis Tiigi --- client/build_test.go | 41 +++++++++++++++++++++---- executor/runcexecutor/executor_linux.go | 29 ++++++++++++++++- 2 files changed, 63 insertions(+), 7 deletions(-) diff --git a/client/build_test.go b/client/build_test.go index 461d6cbcb..1cd04222f 100644 --- a/client/build_test.go +++ b/client/build_test.go @@ -52,6 +52,8 @@ func TestClientGatewayIntegration(t *testing.T) { testNoBuildID, testUnknownBuildID, testClientGatewayContainerExecPipe, + testClientGatewayContainerExecPipeRelease, + testClientGatewayContainerExecPipeSignalKill, testClientGatewayContainerCancelOnRelease, testClientGatewayContainerPID1Fail, testClientGatewayContainerPID1Exit, @@ -398,6 +400,35 @@ func testClientGatewayContainerCancelOnRelease(t *testing.T, sb integration.Sand // process together all started via `Exec` into the same container. // We are mimicing: `echo testing | cat | cat > /tmp/foo && cat /tmp/foo` func testClientGatewayContainerExecPipe(t *testing.T, sb integration.Sandbox) { + testClientGatewayContainerExecPipeWithCleanup(t, sb, func(ctx context.Context, ctr client.Container, pid1 client.ContainerProcess, stdin *io.PipeWriter) { + stdin.Close() + pid1.Wait() + ctr.Release(context.WithoutCancel(ctx)) + }) +} + +// testClientGatewayContainerExecPipeRelease verifies that releasing the +// container while pid1 still has an open stdin reader does not leave runc +// blocked on the stdin copy. +func testClientGatewayContainerExecPipeRelease(t *testing.T, sb integration.Sandbox) { + testClientGatewayContainerExecPipeWithCleanup(t, sb, func(ctx context.Context, ctr client.Container, pid1 client.ContainerProcess, _ *io.PipeWriter) { + ctr.Release(context.WithoutCancel(ctx)) + pid1.Wait() + }) +} + +// testClientGatewayContainerExecPipeSignalKill verifies that SIGKILL delivered +// via the process Signal channel unblocks runc when pid1 still has an open +// stdin reader. +func testClientGatewayContainerExecPipeSignalKill(t *testing.T, sb integration.Sandbox) { + testClientGatewayContainerExecPipeWithCleanup(t, sb, func(ctx context.Context, ctr client.Container, pid1 client.ContainerProcess, _ *io.PipeWriter) { + pid1.Signal(ctx, syscall.SIGKILL) + pid1.Wait() + ctr.Release(context.WithoutCancel(ctx)) + }) +} + +func testClientGatewayContainerExecPipeWithCleanup(t *testing.T, sb integration.Sandbox, cleanup func(ctx context.Context, ctr client.Container, pid1 client.ContainerProcess, stdin *io.PipeWriter)) { requiresLinux(t) ctx := sb.Context() @@ -437,19 +468,17 @@ func testClientGatewayContainerExecPipe(t *testing.T, sb integration.Sandbox) { } // background pid1 process that starts container + pid1StdinR, pid1StdinW := io.Pipe() pid1, err := ctr.Start(ctx, client.StartRequest{ - Args: []string{"sleep", "10"}, + Args: []string{"cat"}, + Stdin: pid1StdinR, }) if err != nil { ctr.Release(context.WithoutCancel(ctx)) return nil, err } - defer func() { - // cancel pid1 - ctr.Release(context.WithoutCancel(ctx)) - pid1.Wait() - }() + defer cleanup(ctx, ctr, pid1, pid1StdinW) // first part is `echo testing | cat` stdin2 := bytes.NewBuffer([]byte("testing")) diff --git a/executor/runcexecutor/executor_linux.go b/executor/runcexecutor/executor_linux.go index cd374b791..f5097e59a 100644 --- a/executor/runcexecutor/executor_linux.go +++ b/executor/runcexecutor/executor_linux.go @@ -83,7 +83,34 @@ func (w *runcExecutor) callWithIO(ctx context.Context, process executor.ProcessI }) if !process.Meta.Tty { - return call(ctx, startedCh, &forwardIO{stdin: process.Stdin, stdout: process.Stdout, stderr: process.Stderr}, killer.pidfile) + runcIO := &forwardIO{stdout: process.Stdout, stderr: process.Stderr} + if process.Stdin != nil { + // Forward stdin through an os.Pipe rather than handing the + // caller's io.ReadCloser to runc directly. When cmd.Stdin is + // an *os.File, exec.Cmd dup2s it into the child and cmd.Wait + // returns as soon as the runc subprocess exits. Otherwise + // exec.Cmd spawns an internal goroutine that blocks on the + // caller's Reader and prevents cmd.Wait from returning after + // the in-container process is killed. Stdin is closed in a + // defer after call() returns, matching the tty path and the + // natural-exit cleanup. + pr, pw, err := os.Pipe() + if err != nil { + return errors.Wrap(err, "failed to create stdin pipe") + } + runcIO.stdin = pr + defer pr.Close() + defer process.Stdin.Close() + eg.Go(func() error { + defer pw.Close() + _, err := io.Copy(pw, process.Stdin) + if errors.Is(err, io.ErrClosedPipe) || errors.Is(err, os.ErrClosed) { + return nil + } + return err + }) + } + return call(ctx, startedCh, runcIO, killer.pidfile) } ptm, ptsName, err := console.NewPty()