fix: align process handling and fix Windows crashes (#1376)

The intermittent Windows test crashes came from step errors formatting a nil `rc.Run`. `fmt` recovers the nil dereference, but on CPUs with AMX the recovered fault overflows into the heap (https://github.com/golang/go/issues/81238). The errors now name the job, and the Windows test VM hides AMX and runs on UTC, so cached boots no longer jump back 8 hours mid-test.

Windows commands now start suspended and resume once they are in their job objects, so no child escapes them. That drops the process scan reading other processes' memory and the last `unsafe` code, which `depguard` now denies. Step processes outlive a crashed runner, and commands are no longer looked up in the runner's working directory, both as with GitHub's runner.

Like GitHub's runner, host jobs on Linux and macOS now end the processes carrying the job's `RUNNER_TRACKING_ID`, and image pulls and Dockerfile action builds are retried.

Reviewed-on: https://gitea.com/gitea/runner/pulls/1376
Reviewed-by: bircni <[email protected]>
Co-authored-by: silverwind <[email protected]>

act/common/executor.go

@@ -7,7 +7,9 @@ package common
 import (
 	"context"
 	"fmt"
+	"math/rand/v2"
 	"runtime/debug"
+	"time"
 
 	"gitea.com/gitea/runner/act/common/logging"
 )
@@ -158,6 +160,28 @@ func (e Executor) IfBool(conditional bool) Executor {
 	})
 }
 
+// RetryBackoff is the wait between Retry's attempts, random between 1 and 10 seconds like GitHub's runner.
+var RetryBackoff = func() time.Duration { return time.Second + rand.N(9*time.Second) }
+
+// Retry runs the executor up to attempts times until it succeeds.
+func (e Executor) Retry(attempts int) Executor {
+	return func(ctx context.Context) error {
+		for attempt := 1; ; attempt++ {
+			err := e(ctx)
+			if err == nil || attempt == attempts || ctx.Err() != nil {
+				return err
+			}
+			backoff := RetryBackoff()
+			Logger(ctx).Warnf("%v, retrying in %s", err, backoff.Round(time.Second))
+			select {
+			case <-ctx.Done():
+				return err
+			case <-time.After(backoff):
+			}
+		}
+	}
+}
+
 // Finally adds an executor to run after other executor
 func (e Executor) Finally(finally Executor) Executor {
 	return func(ctx context.Context) error {

act/common/executor_test.go

@@ -11,6 +11,7 @@ import (
 	"sync"
 	"sync/atomic"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -134,6 +135,31 @@ func TestExecutorConditionalsAndFinally(t *testing.T) {
 	assert.Equal(t, []string{"if-true", "if-not", "if-bool", "main", "finally"}, calls)
 }
 
+func TestExecutorRetryStopsOnSuccessOrAfterAllAttempts(t *testing.T) {
+	backoff := RetryBackoff
+	RetryBackoff = func() time.Duration { return 0 }
+	t.Cleanup(func() { RetryBackoff = backoff })
+	transient := errors.New("transient")
+	for _, tc := range []struct {
+		failures, calls int
+		err             error
+	}{
+		{failures: 2, calls: 3},
+		{failures: 5, calls: 3, err: transient},
+	} {
+		var calls int
+		err := Executor(func(context.Context) error {
+			calls++
+			if calls <= tc.failures {
+				return transient
+			}
+			return nil
+		}).Retry(3)(t.Context())
+		require.Equal(t, tc.err, err)
+		require.Equal(t, tc.calls, calls)
+	}
+}
+
 func TestExecutorFinallyReturnsFinallyErrorWithOriginal(t *testing.T) {
 	finalErr := errors.New("cleanup failed")
 	for _, mainErr := range []error{context.Canceled, nil} {

act/container/docker_pull.go

@@ -22,7 +22,7 @@ import (
 
 // NewDockerPullExecutor function to create a run executor for the container
 func NewDockerPullExecutor(input NewDockerPullExecutorInput) common.Executor {
-	return func(ctx context.Context) error {
+	return common.Executor(func(ctx context.Context) error {
 		logger := common.Logger(ctx)
 		logger.Debugf("docker pull %v", input.Image)
 
@@ -83,7 +83,7 @@ func NewDockerPullExecutor(input NewDockerPullExecutorInput) common.Executor {
 			return nil
 		}
 		return fmt.Errorf("failed to pull image '%s' (%s): %w", imageRef, input.Platform, err)
-	}
+	}).Retry(3)
 }
 
 func getImagePullOptions(ctx context.Context, input NewDockerPullExecutorInput) (client.ImagePullOptions, error) {

act/container/host_environment.go

@@ -57,7 +57,7 @@ func (e *HostEnvironment) processGroup(ctx context.Context) *process.Group {
 	e.procGroupOnce.Do(func() {
 		group, err := process.NewGroup()
 		if err != nil {
-			common.Logger(ctx).Warnf("could not create the job's process group; processes a step leaves behind can only be reclaimed by the workspace scan: %v", err)
+			common.Logger(ctx).Warnf("could not create the job's process group, processes a step leaves behind may outlive the job: %v", err)
 			return
 		}
 		e.procGroup.Store(group)
@@ -415,46 +415,6 @@ func removePathWithRetry(ctx context.Context, path string) error {
 	return lastErr
 }
 
-func (e *HostEnvironment) terminateRunningProcesses(ctx context.Context) {
-	if runtime.GOOS != "windows" {
-		return
-	}
-
-	// Detached: exec.CommandContext won't start on a cancelled ctx, and a
-	// server cancel has already cancelled the parent ctx.
-	killCtx, killCancel := context.WithTimeout(context.Background(), 30*time.Second)
-	defer killCancel()
-
-	logger := common.Logger(ctx)
-
-	// Dirs we own; a process referencing one is a leftover. ToolCache is shared
-	// across jobs, and Workdir may be a caller-owned checkout.
-	owned := []string{e.Path, e.TmpDir}
-	if e.CleanWorkdir {
-		owned = append(owned, e.Workdir)
-	}
-	dirs := make([]string, 0, len(owned))
-	for _, d := range owned {
-		if d == "" {
-			continue
-		}
-		abs, err := filepath.Abs(d)
-		if err != nil {
-			continue
-		}
-		dirs = append(dirs, abs)
-	}
-	if len(dirs) == 0 {
-		return
-	}
-
-	if killed, err := process.KillProcessesWithPaths(killCtx, dirs, e.TrackingID); err != nil {
-		logger.Debugf("workspace process cleanup reported errors: %v", err)
-	} else if killed > 0 {
-		logger.Debugf("terminated %d leftover workspace process(es)", killed)
-	}
-}
-
 // hostCleanupTimeout bounds each teardown phase so one stalled delete cannot
 // wedge the runner slot. A var so tests can shrink it.
 var hostCleanupTimeout = 30 * time.Second
@@ -463,12 +423,15 @@ func (e *HostEnvironment) Remove() common.Executor {
 	return func(ctx context.Context) error {
 		logger := common.Logger(ctx)
 
-		// End lingering processes before removing the workspace; on Windows their
-		// file locks block cleanup. Closing the group is deterministic, the scan a net.
+		// End lingering processes before removing the workspace, on Windows their file locks block cleanup.
 		if err := e.procGroup.Load().Close(); err != nil {
 			logger.Debugf("closing the job's process group failed: %v", err)
 		}
-		e.terminateRunningProcesses(ctx)
+		if killed, err := process.KillTrackedProcesses(e.TrackingID); err != nil {
+			logger.Debugf("killing the job's leftover processes reported errors: %v", err)
+		} else if killed > 0 {
+			logger.Debugf("killed %d leftover process(es) of the job", killed)
+		}
 
 		// Detach: a cancelled ctx would skip removePathWithRetry's retries,
 		// which absorb Windows file-handle release lag after the kill above.

act/container/host_environment_windows_test.go

@@ -4,18 +4,15 @@
 package container
 
 import (
-	"bufio"
 	"context"
 	"fmt"
 	"io"
 	"net"
 	"os"
 	"os/exec"
-	"path/filepath"
 	"strings"
 	"testing"
 	"time"
-	"unsafe"
 
 	"github.com/stretchr/testify/require"
 	"golang.org/x/sys/windows"
@@ -40,7 +37,6 @@ func TestHostEnvironmentTrackingCleanup(t *testing.T) {
 				}
 			}
 			env["GITEA_TRACKING_HELPER"] = "step"
-			env["GITEA_TRACKING_WORKSPACE"] = workspace
 			env["GITEA_TRACKING_ADDRESS"] = listener.Addr().String()
 			if tracking == "default" {
 				env["RUNNER_TRACKING_ID"] = environment.TrackingID
@@ -52,23 +48,6 @@ func TestHostEnvironmentTrackingCleanup(t *testing.T) {
 			go func() {
 				result <- environment.Exec([]string{os.Args[0], "-test.run=^TestHostTrackingHelper$"}, env, "", workspace)(ctx)
 			}()
-			step, err := listener.Accept()
-			require.NoError(t, err)
-			defer step.Close()
-			require.NoError(t, step.SetDeadline(time.Now().Add(20*time.Second)))
-			var stepPID int
-			_, err = fmt.Fscanln(step, &stepPID)
-			require.NoError(t, err)
-			stepHandle, err := windows.OpenProcess(windows.PROCESS_QUERY_LIMITED_INFORMATION, false, uint32(stepPID))
-			require.NoError(t, err)
-			defer func() { _ = windows.CloseHandle(stepHandle) }()
-			require.Eventually(t, func() bool {
-				var assigned int32
-				result, _, _ := windows.NewLazySystemDLL("kernel32.dll").NewProc("IsProcessInJob").Call(uintptr(stepHandle), 0, uintptr(unsafe.Pointer(&assigned)))
-				return result != 0 && assigned != 0
-			}, 5*time.Second, 10*time.Millisecond)
-			_, err = step.Write([]byte{1})
-			require.NoError(t, err)
 			var helpers []windows.Handle
 			for range 2 {
 				connection, err := listener.Accept()
@@ -114,23 +93,18 @@ func TestHostTrackingHelper(t *testing.T) {
 	if mode == "" {
 		return
 	}
+	if mode != "descendant" {
+		child := exec.Command(os.Args[0], "-test.run=^TestHostTrackingHelper$")
+		child.Env = append(os.Environ(), "GITEA_TRACKING_HELPER="+map[string]string{"step": "helper", "helper": "descendant"}[mode])
+		require.NoError(t, child.Start())
+		if mode == "step" {
+			os.Exit(0)
+		}
+	}
 	connection, err := net.Dial("tcp4", os.Getenv("GITEA_TRACKING_ADDRESS"))
 	require.NoError(t, err)
 	_, err = fmt.Fprintln(connection, os.Getpid())
 	require.NoError(t, err)
-	if mode == "step" {
-		_, err := bufio.NewReader(connection).ReadByte()
-		require.NoError(t, err)
-	}
-	if mode != "descendant" {
-		child := exec.Command(os.Args[0], "-test.run=^TestHostTrackingHelper$", filepath.Join(os.Getenv("GITEA_TRACKING_WORKSPACE"), "tracking-reference"))
-		child.Env = append(os.Environ(), "GITEA_TRACKING_HELPER="+map[string]string{"step": "helper", "helper": "descendant"}[mode])
-		require.NoError(t, child.Start())
-		if mode == "step" {
-			require.NoError(t, connection.Close())
-			os.Exit(0)
-		}
-	}
 	_, err = io.Copy(io.Discard, connection)
 	require.NoError(t, err)
 	require.NoError(t, connection.Close())

act/lookpath/lp_windows.go

@@ -80,9 +80,6 @@ func lookPath(file string, env map[string]string) (string, error) {
 			return "", &Error{file, err}
 		}
 	}
-	if f, err := findExecutable(filepath.Join(".", file), exts); err == nil {
-		return f, nil
-	}
 	path := getenv(env, "path")
 	for _, dir := range filepath.SplitList(path) {
 		if f, err := findExecutable(filepath.Join(dir, file), exts); err == nil {

act/lookpath/lp_windows_test.go

@@ -0,0 +1,20 @@
+// Copyright 2026 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package lookpath
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestLookPath2IgnoresTheCurrentDirectory(t *testing.T) {
+	dir := t.TempDir()
+	require.NoError(t, os.WriteFile(filepath.Join(dir, "tool.exe"), nil, 0o755))
+	t.Chdir(dir)
+	_, err := LookPath2("tool", map[string]string{"PATH": t.TempDir()})
+	require.EqualError(t, err, ErrNotFound.Error())
+}

act/runner/action.go

@@ -339,34 +339,30 @@ func execAsDocker(ctx context.Context, step actionStep, actionName, actionDir, b
 
 		if !correctArchExists || rc.Config.ForceRebuild {
 			logger.Debugf("image '%s' for architecture '%s' will be built from context '%s", image, rc.Config.ContainerArchitecture, contextDir)
-			var buildContext io.ReadCloser
-			if localAction {
-				buildContext, err = rc.JobContainer.GetContainerArchive(ctx, contextDir+"/.")
-				if err != nil {
-					return err
+			prepImage = common.Executor(func(ctx context.Context) error {
+				input := container.NewDockerBuildExecutorInput{
+					ContextDir: contextDir,
+					Dockerfile: fileName,
+					ImageTag:   image,
+					Platform:   rc.Config.ContainerArchitecture,
+					BuildArgs:  rc.proxyBuildArgs(),
 				}
-				defer buildContext.Close()
-			}
-			prepImage = ContainerNewDockerBuildExecutor(container.NewDockerBuildExecutorInput{
-				ContextDir:   contextDir,
-				Dockerfile:   fileName,
-				ImageTag:     image,
-				BuildContext: buildContext,
-				Platform:     rc.Config.ContainerArchitecture,
-				BuildArgs:    rc.proxyBuildArgs(),
-			})
-			if buildContext == nil {
-				// Held across the whole build: the daemon drains contextDir lazily.
-				inner := prepImage
-				prepImage = func(ctx context.Context) error {
-					unlock, err := git.AcquireCloneLock(ctx, actionDir)
+				if localAction {
+					buildContext, err := rc.JobContainer.GetContainerArchive(ctx, contextDir+"/.")
+					if err != nil {
+						return err
+					}
+					defer buildContext.Close()
+					input.BuildContext = buildContext
+				} else {
+					unlock, err := git.AcquireCloneLock(ctx, actionDir) // held across the whole build, the daemon drains contextDir lazily
 					if err != nil {
 						return err
 					}
 					defer unlock()
-					return inner(ctx)
 				}
-			}
+				return ContainerNewDockerBuildExecutor(input)(ctx)
+			}).Retry(3)
 		} else {
 			logger.Debugf("image '%s' for architecture '%s' already exists", image, rc.Config.ContainerArchitecture)
 		}

act/runner/step_factory.go

@@ -22,7 +22,7 @@ type stepFactoryImpl struct{}
 func (sf *stepFactoryImpl) newStep(stepModel *model.Step, rc *RunContext) (step, error) {
 	switch stepModel.Type() {
 	case model.StepTypeInvalid:
-		return nil, fmt.Errorf("invalid run/uses syntax for job:%s step:%+v", rc.Run, stepModel)
+		return nil, fmt.Errorf("invalid run/uses syntax for job:%s step:%+v", rc.Name, stepModel)
 	case model.StepTypeRun:
 		return &stepRun{
 			Step:       stepModel,
@@ -40,7 +40,7 @@ func (sf *stepFactoryImpl) newStep(stepModel *model.Step, rc *RunContext) (step,
 			run, ok := builtinActions[name]
 			if !ok {
 				return nil, fmt.Errorf("unknown built-in action %q (known: %v) for job:%s step:%+v",
-					name, slices.Sorted(maps.Keys(builtinActions)), rc.Run, stepModel)
+					name, slices.Sorted(maps.Keys(builtinActions)), rc.Name, stepModel)
 			}
 			return &stepActionBuiltin{
 				Step:       stepModel,
@@ -61,5 +61,5 @@ func (sf *stepFactoryImpl) newStep(stepModel *model.Step, rc *RunContext) (step,
 		}, nil
 	}
 
-	return nil, fmt.Errorf("unable to determine how to run job:%s step:%+v", rc.Run, stepModel)
+	return nil, fmt.Errorf("unable to determine how to run job:%s step:%+v", rc.Name, stepModel)
 }

act/runner/step_factory_test.go

@@ -37,7 +37,7 @@ func TestStepFactoryInvalidStep(t *testing.T) {
 		{Uses: "remote/action@v1", Run: "cmd"},
 		{Uses: "builtin:unknown"},
 	} {
-		_, err := (&stepFactoryImpl{}).newStep(stepModel, &RunContext{})
-		assert.Error(t, err, stepModel.Uses)
+		_, err := (&stepFactoryImpl{}).newStep(stepModel, &RunContext{Name: "build"})
+		assert.ErrorContains(t, err, "job:build ", stepModel.Uses)
 	}
 }