DevFW-CICD/runner
15
0
Fork 0
Code Issues Pull requests Projects Releases 1 Packages Wiki Activity Actions

chore: prevent 'false positive' data race detection with Job.If

Browse source 
Initialization of the default value of the `If` field is functionally
safe, but triggers the data race detector.

==================
WARNING: DATA RACE
Read at 0x00c00037cf78 by goroutine 10:
  code.forgejo.org/forgejo/runner/v9/act/model.(*Workflow).GetJob()
      /.../forgejo-runner/act/model/workflow.go:766 +0x2ae
  code.forgejo.org/forgejo/runner/v9/act/model.(*Run).Job()
      /.../forgejo-runner/act/model/planner.go:50 +0xab
  code.forgejo.org/forgejo/runner/v9/act/runner.setJobResult()
      /.../forgejo-runner/act/runner/job_executor.go:168
+0x7c
  code.forgejo.org/forgejo/runner/v9/act/runner.TestSetJobResultConcurrency.func3()
      /.../forgejo-runner/act/runner/job_executor_test.go:410
+0xf8

Previous write at 0x00c00037cf78 by goroutine 9:
  code.forgejo.org/forgejo/runner/v9/act/model.(*Workflow).GetJob()
      /.../forgejo-runner/act/model/workflow.go:767 +0x2ce
  code.forgejo.org/forgejo/runner/v9/act/model.(*Run).Job()
      /.../forgejo-runner/act/model/planner.go:50 +0xab
  code.forgejo.org/forgejo/runner/v9/act/runner.setJobResult()
      /.../forgejo-runner/act/runner/job_executor.go:168
+0x7c
  code.forgejo.org/forgejo/runner/v9/act/runner.TestSetJobResultConcurrency.func2()
      /.../forgejo-runner/act/runner/job_executor_test.go:405
+0xfb

Goroutine 10 (running) created at:
  code.forgejo.org/forgejo/runner/v9/act/runner.TestSetJobResultConcurrency()
      /.../forgejo-runner/act/runner/job_executor_test.go:408
+0xbac
  testing.tRunner()
      /home/mfenniak/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.6.linux-amd64/src/testing/testing.go:1792
+0x225
  testing.(*T).Run.gowrap1()
      /home/mfenniak/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.6.linux-amd64/src/testing/testing.go:1851
+0x44

Goroutine 9 (running) created at:
  code.forgejo.org/forgejo/runner/v9/act/runner.TestSetJobResultConcurrency()
      /.../forgejo-runner/act/runner/job_executor_test.go:403
+0xa84
  testing.tRunner()
      /home/mfenniak/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.6.linux-amd64/src/testing/testing.go:1792
+0x225
  testing.(*T).Run.gowrap1()
      /home/mfenniak/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.6.linux-amd64/src/testing/testing.go:1851
+0x44
==================
This commit is contained in:
Mathieu Fenniak 2025-08-14 20:10:09 -06:00
parent 886bf2a4f3
commit fb9b6ada91
3 changed files with 12 additions and 8 deletions
Download patch file Download diff file Expand all files Collapse all files

12
act/model/workflow.go
View file

@ -206,7 +206,7 @@ type Job struct {
RawNeeds yaml.Node `yaml:"needs"`
RawRunsOn yaml.Node `yaml:"runs-on"`
Env yaml.Node `yaml:"env"`
If yaml.Node `yaml:"if"`
ifClause yaml.Node `yaml:"if"`
Steps []*Step `yaml:"steps"`
TimeoutMinutes string `yaml:"timeout-minutes"`
Services map[string]*ContainerSpec `yaml:"services"`
@ -359,6 +359,13 @@ func (j *Job) RunsOn() []string {
}
}
func (j *Job) IfClause() string {
if j.ifClause.Value == "" {
return "success()"
}
return j.ifClause.Value
}
func nodeAsStringSlice(node yaml.Node) []string {
switch node.Kind {
case yaml.ScalarNode:
@ -760,9 +767,6 @@ func (w *Workflow) GetJob(jobID string) *Job {
if j.Name == "" {
j.Name = id
}
if j.If.Value == "" {
j.If.Value = "success()"
}
return j
}
}

6
act/runner/run_context.go
View file

@ -1025,11 +1025,11 @@ func (rc *RunContext) options(ctx context.Context) string {
func (rc *RunContext) isEnabled(ctx context.Context) (bool, error) {
job := rc.Run.Job()
l := common.Logger(ctx)
runJob, runJobErr := EvalBool(ctx, rc.ExprEval, job.If.Value, exprparser.DefaultStatusCheckSuccess)
runJob, runJobErr := EvalBool(ctx, rc.ExprEval, job.IfClause(), exprparser.DefaultStatusCheckSuccess)
jobType, jobTypeErr := job.Type()
if runJobErr != nil {
return false, fmt.Errorf(" \u274C Error in if-expression: \"if: %s\" (%s)", job.If.Value, runJobErr)
return false, fmt.Errorf(" \u274C Error in if-expression: \"if: %s\" (%s)", job.IfClause(), runJobErr)
}
if jobType == model.JobTypeInvalid {
@ -1038,7 +1038,7 @@ func (rc *RunContext) isEnabled(ctx context.Context) (bool, error) {
if !runJob {
rc.result("skipped")
l.WithField("jobResult", "skipped").Infof("Skipping job '%s' due to '%s'", job.Name, job.If.Value)
l.WithField("jobResult", "skipped").Infof("Skipping job '%s' due to '%s'", job.Name, job.IfClause())
return false, nil
}

2
act/runner/runner.go
View file

@ -147,7 +147,7 @@ func (runner *runnerImpl) NewPlanExecutor(plan *model.Plan) common.Executor {
log.Debugf("Job.RawNeeds: %v", job.RawNeeds)
log.Debugf("Job.RawRunsOn: %v", job.RawRunsOn)
log.Debugf("Job.Env: %v", job.Env)
log.Debugf("Job.If: %v", job.If)
log.Debugf("Job.If: %v", job.IfClause())
for step := range job.Steps {
if nil != job.Steps[step] {
log.Debugf("Job.Steps: %v", job.Steps[step].String())