diff --git a/act/model/workflow.go b/act/model/workflow.go index 82082feb..3462d288 100644 --- a/act/model/workflow.go +++ b/act/model/workflow.go @@ -11,6 +11,7 @@ import ( "slices" "strconv" "strings" + "sync" "code.forgejo.org/forgejo/runner/v9/act/common" "code.forgejo.org/forgejo/runner/v9/act/schema" @@ -217,7 +218,9 @@ type Job struct { Uses string `yaml:"uses"` With map[string]any `yaml:"with"` RawSecrets yaml.Node `yaml:"secrets"` - Result string + + Result string + ResultMutex sync.Mutex } // Strategy for the job diff --git a/act/runner/job_executor.go b/act/runner/job_executor.go index ed1f3004..3cb4f206 100644 --- a/act/runner/job_executor.go +++ b/act/runner/job_executor.go @@ -164,8 +164,8 @@ func setJobResult(ctx context.Context, info jobInfo, rc *RunContext, success boo // As we're reading the matrix build's status (`rc.Run.Job().Result`), it's possible for it change in another // goroutine running `setJobResult` and invoking `.result(...)`. Prevent concurrent execution of `setJobResult`... - rc.JobResultMutex.Lock() - defer rc.JobResultMutex.Unlock() + rc.Run.Job().ResultMutex.Lock() + defer rc.Run.Job().ResultMutex.Unlock() jobResult := "success" // we have only one result for a whole matrix build, so we need diff --git a/act/runner/job_executor_test.go b/act/runner/job_executor_test.go index 0c0ad89c..3915b971 100644 --- a/act/runner/job_executor_test.go +++ b/act/runner/job_executor_test.go @@ -338,14 +338,26 @@ func TestJobExecutorNewJobExecutor(t *testing.T) { func TestSetJobResultConcurrency(t *testing.T) { jim := &jobInfoMock{} - rc := &RunContext{ + job := model.Job{ + Result: "success", + } + // Distinct RunContext objects are used to replicate realistic setJobResult in matrix build + rc1 := &RunContext{ Run: &model.Run{ JobID: "test", Workflow: &model.Workflow{ Jobs: map[string]*model.Job{ - "test": { - Result: "success", - }, + "test": &job, + }, + }, + }, + } + rc2 := &RunContext{ + Run: &model.Run{ + JobID: "test", + Workflow: &model.Workflow{ + Jobs: map[string]*model.Job{ + "test": &job, }, }, }, @@ -374,7 +386,7 @@ func TestSetJobResultConcurrency(t *testing.T) { if result == "success" { time.Sleep(1 * time.Second) } - rc.Run.Job().Result = result + job.Result = result lastResult = result }) @@ -383,12 +395,12 @@ func TestSetJobResultConcurrency(t *testing.T) { // Goroutine 1, mark as success: go func() { defer wg.Done() - setJobResult(t.Context(), jim, rc, true) + setJobResult(t.Context(), jim, rc1, true) }() // Goroutine 2, mark as failure: go func() { defer wg.Done() - setJobResult(t.Context(), jim, rc, false) + setJobResult(t.Context(), jim, rc2, false) }() wg.Wait() diff --git a/act/runner/run_context.go b/act/runner/run_context.go index fbef99a3..5ed2bb35 100644 --- a/act/runner/run_context.go +++ b/act/runner/run_context.go @@ -18,7 +18,6 @@ import ( "regexp" "runtime" "strings" - "sync" "text/template" "time" @@ -57,8 +56,6 @@ type RunContext struct { randomName string networkName string networkCreated bool - - JobResultMutex sync.Mutex } func (rc *RunContext) AddMask(mask string) {