Kill timed-out check process groups

Author: andrewxhill-ai

internal/app/checks.go

@@ -1,6 +1,7 @@
 package app
 
 import (
+	"bytes"
 	"context"
 	"errors"
 	"fmt"
@@ -59,16 +60,12 @@ func runConfiguredChecks(checks []string, workdir string, timeoutSetting string)
 		}
 
 		ctx, cancel := context.WithTimeout(context.Background(), effectiveTimeout)
-		cmd := exec.CommandContext(ctx, "/bin/sh", "-lc", command)
-		cmd.Dir = workdir
-		output, err := cmd.CombinedOutput()
+		output, err := runTimedCheck(ctx, workdir, command)
 		cancel()
-		if ctx.Err() == context.DeadlineExceeded {
-			return &checkTimeoutError{
-				Command:          command,
-				RequestedTimeout: timeout,
-				EffectiveTimeout: effectiveTimeout,
-			}
+		if timeoutErr, ok := isCheckTimeoutError(err); ok {
+			timeoutErr.RequestedTimeout = timeout
+			timeoutErr.EffectiveTimeout = effectiveTimeout
+			return timeoutErr
 		}
 		if err != nil {
 			return fmt.Errorf("check %q failed: %s", command, strings.TrimSpace(string(output)))
@@ -78,6 +75,37 @@ func runConfiguredChecks(checks []string, workdir string, timeoutSetting string)
 	return nil
 }
 
+func runTimedCheck(ctx context.Context, workdir string, command string) ([]byte, error) {
+	cmd := exec.Command("/bin/sh", "-lc", command)
+	cmd.SysProcAttr = checkSysProcAttr()
+	cmd.Dir = workdir
+	var output bytes.Buffer
+	cmd.Stdout = &output
+	cmd.Stderr = &output
+	if err := cmd.Start(); err != nil {
+		return nil, err
+	}
+
+	resultCh := make(chan error, 1)
+	go func() {
+		resultCh <- cmd.Wait()
+	}()
+
+	select {
+	case err := <-resultCh:
+		return output.Bytes(), err
+	case <-ctx.Done():
+		_ = interruptCheckProcess(cmd.Process.Pid)
+		err := <-resultCh
+		if ctx.Err() == context.DeadlineExceeded {
+			return nil, &checkTimeoutError{
+				Command: command,
+			}
+		}
+		return output.Bytes(), err
+	}
+}
+
 func resolveCommandTimeout(timeoutSetting string) (time.Duration, time.Duration, error) {
 	timeout, err := time.ParseDuration(timeoutSetting)
 	if err != nil {

internal/app/checks_unix_test.go

@@ -0,0 +1,59 @@
+//go:build unix
+
+package app
+
+import (
+	"os"
+	"path/filepath"
+	"strconv"
+	"strings"
+	"syscall"
+	"testing"
+	"time"
+)
+
+func TestRunConfiguredChecksTimeoutKillsChildProcessGroup(t *testing.T) {
+	workdir := t.TempDir()
+	pidFile := filepath.Join(workdir, "child.pid")
+
+	err := runConfiguredChecks([]string{
+		"sleep 30 & child=$!; echo $child > child.pid; wait $child",
+	}, workdir, "100ms")
+	timeoutErr, ok := isCheckTimeoutError(err)
+	if !ok {
+		t.Fatalf("expected check timeout error, got %v", err)
+	}
+	if timeoutErr == nil {
+		t.Fatalf("expected timeout error details")
+	}
+
+	var pid int
+	deadline := time.Now().Add(2 * time.Second)
+	for {
+		data, readErr := os.ReadFile(pidFile)
+		if readErr == nil {
+			parsedPID, convErr := strconv.Atoi(strings.TrimSpace(string(data)))
+			if convErr != nil {
+				t.Fatalf("parse child pid: %v", convErr)
+			}
+			pid = parsedPID
+			break
+		}
+		if time.Now().After(deadline) {
+			t.Fatalf("timed out waiting for child pid file: %v", readErr)
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+
+	deadline = time.Now().Add(2 * time.Second)
+	for {
+		killErr := syscall.Kill(pid, 0)
+		if killErr != nil {
+			return
+		}
+		if time.Now().After(deadline) {
+			t.Fatalf("timed out waiting for child pid %d to exit", pid)
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+}

internal/app/process_default.go

@@ -0,0 +1,20 @@
+//go:build !unix
+
+package app
+
+import (
+	"os"
+	"syscall"
+)
+
+func checkSysProcAttr() *syscall.SysProcAttr {
+	return nil
+}
+
+func interruptCheckProcess(pid int) error {
+	process, err := os.FindProcess(pid)
+	if err != nil {
+		return err
+	}
+	return process.Kill()
+}

internal/app/process_unix.go

@@ -0,0 +1,13 @@
+//go:build unix
+
+package app
+
+import "syscall"
+
+func checkSysProcAttr() *syscall.SysProcAttr {
+	return &syscall.SysProcAttr{Setpgid: true}
+}
+
+func interruptCheckProcess(pid int) error {
+	return syscall.Kill(-pid, syscall.SIGKILL)
+}