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 <bircni@icloud.com>
Reviewed-on: https://gitea.com/gitea/runner/pulls/1323
Reviewed-by: bircni <bircni@icloud.com>
Co-authored-by: silverwind <me@silverwind.io>

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())

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
+}

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)