From ed940db4f0cb81d397ae7711a21d11426d2f94f4 Mon Sep 17 00:00:00 2001 From: silverwind Date: Sat, 17 Oct 2026 09:03:48 +0000 Subject: [PATCH] fix: don't fail host steps over a leftover holding their output (#1323) A successful host step whose background child kept its output open failed with `exec: WaitDelay expired before I/O complete`. Like GitHub's runner (`ProcessExitedHandler` in https://github.com/actions/runner/blob/main/src/Runner.Sdk/ProcessInvoker.cs), the step now succeeds after the drain timeout. The leftover is killed only on Windows, as GitHub does there. A step whose timeout or cancellation hits during the drain still fails. Fixes: https://gitea.com/gitea/runner/issues/1290 Co-authored-by: bircni Reviewed-on: https://gitea.com/gitea/runner/pulls/1323 Reviewed-by: bircni Co-authored-by: silverwind --- act/container/host_environment.go | 2 +- internal/pkg/process/treekill.go | 15 +++++++++++ internal/pkg/process/treekill_test.go | 39 +++++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 1 deletion(-) diff --git a/act/container/host_environment.go b/act/container/host_environment.go index 2e085667..5ed3b43a 100644 --- a/act/container/host_environment.go +++ b/act/container/host_environment.go @@ -338,7 +338,7 @@ func (e *HostEnvironment) exec(ctx context.Context, command []string, cmdline st } else { defer k.Close() } - err = cmd.Wait() + err = treeKill.Wait(ctx, cmd) if err != nil { if exitErr, ok := errors.AsType[*exec.ExitError](err); ok { return ExitCodeError(exitErr.ExitCode()) diff --git a/internal/pkg/process/treekill.go b/internal/pkg/process/treekill.go index bd478c14..2073253e 100644 --- a/internal/pkg/process/treekill.go +++ b/internal/pkg/process/treekill.go @@ -4,8 +4,11 @@ package process import ( + "context" + "errors" "os" "os/exec" + "runtime" "sync/atomic" "time" ) @@ -64,3 +67,15 @@ func (t *TreeKill) Capture(p *os.Process) (*Killer, error) { t.killer.Store(k) return k, nil } + +// Wait is cmd.Wait, but like GitHub's runner a leftover holding a successful command's output does not fail it, and is killed only on Windows. +func (t *TreeKill) Wait(ctx context.Context, cmd *exec.Cmd) error { + err := cmd.Wait() + if !errors.Is(err, exec.ErrWaitDelay) || ctx.Err() != nil { + return err + } + if runtime.GOOS == "windows" { + _ = t.killer.Load().Kill() + } + return nil +} diff --git a/internal/pkg/process/treekill_test.go b/internal/pkg/process/treekill_test.go index 53b99251..099963fb 100644 --- a/internal/pkg/process/treekill_test.go +++ b/internal/pkg/process/treekill_test.go @@ -4,8 +4,12 @@ package process import ( + "context" + "io" "os/exec" + "runtime" "testing" + "time" "github.com/stretchr/testify/require" ) @@ -20,6 +24,41 @@ func TestNewTreeKillConfiguresCommand(t *testing.T) { require.NoError(t, cmd.Cancel()) } +func TestTreeKillWaitWithLeftoverHoldingOutput(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("uses POSIX shell") + } + for _, tc := range []struct { + name string + timeout time.Duration + wantErr bool + }{ + {name: "succeeds", timeout: time.Minute}, + {name: "fails once the context is done", timeout: 10 * time.Millisecond, wantErr: true}, + } { + t.Run(tc.name, func(t *testing.T) { + ctx, cancel := context.WithTimeout(t.Context(), tc.timeout) + defer cancel() + cmd := exec.CommandContext(ctx, "sh", "-c", "sleep 30 &") + cmd.Stdout = io.Discard + cmd.SysProcAttr = SysProcAttr("", false) + tk := NewTreeKill(cmd) + cmd.WaitDelay = 50 * time.Millisecond + require.NoError(t, cmd.Start()) + killer, err := tk.Capture(cmd.Process) + require.NoError(t, err) + t.Cleanup(func() { _ = killer.Kill() }) + + err = tk.Wait(ctx, cmd) + if tc.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +} + func TestTreeKillCaptureStoresKiller(t *testing.T) { cmd := exec.CommandContext(t.Context(), "sleep", "10") cmd.SysProcAttr = SysProcAttr("", false)