1
0
Fork 0
mirror of https://code.forgejo.org/forgejo/runner.git synced 2025-09-05 18:40:59 +00:00
forgejo-runner/act/model
Mathieu Fenniak 165b44deec
fix: data race in 'runs-on' expressions causes incorrect job labels during execution (#871)
A job with a `runs-on` that references matrix variables will not run with the expected labels.  eg.

```
jobs:
  matrix-runs-on:
    strategy:
      matrix:
        os: [ubuntu-latest, ubuntu-20.04]
    runs-on: ${{ matrix.os }}
    steps:
      ...
```
Due to shared mutated state, both jobs that this generates will (w/ a race condition) either run with the `ubuntu-latest` or `ubuntu-20.04`, but rarely (never observed) with the expected outcome of running on both labels.

`EvaluateYamlNode` is used to evaluate expressions in the `runs-on` field in the context of the current running job, but mutating an object shared between multiple concurrent jobs (in matrix evaluation).  This results in the evaluation results from one job spilling into another and corrupting their `runs-on` labels.

```
==================
WARNING: DATA RACE
Write at 0x00c00047e0b0 by goroutine 1739:
  reflect.typedmemmove()
      /.../go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.6.linux-amd64/src/runtime/mbarrier.go:213 +0x0
  reflect.Value.Set()
      /.../go/pkg/mod/golang.org/toolchain@v0.0.1-go1.24.6.linux-amd64/src/reflect/value.go:2062 +0x184
  gopkg.in/yaml%2ev3.(*decoder).unmarshal()
      /.../go/pkg/mod/gopkg.in/yaml.v3@v3.0.1/decode.go:493 +0x7b4
  gopkg.in/yaml%2ev3.(*Node).Decode()
      /.../go/pkg/mod/gopkg.in/yaml.v3@v3.0.1/yaml.go:149 +0x355
  code.forgejo.org/forgejo/runner/v9/act/runner.expressionEvaluator.EvaluateYamlNode()
      /.../forgejo-runner/act/runner/expression.go:372 +0x7a
  code.forgejo.org/forgejo/runner/v9/act/runner.(*expressionEvaluator).EvaluateYamlNode()
      <autogenerated>:1 +0x6b
  code.forgejo.org/forgejo/runner/v9/act/runner.(*RunContext).runsOnPlatformNames()
      /.../forgejo-runner/act/runner/run_context.go:1019 +0x2af
  code.forgejo.org/forgejo/runner/v9/act/runner.(*RunContext).runsOnImage()
      /.../forgejo-runner/act/runner/run_context.go:1002 +0x772
  code.forgejo.org/forgejo/runner/v9/act/runner.(*RunContext).platformImage()
      /.../forgejo-runner/act/runner/run_context.go:1032 +0x77
  code.forgejo.org/forgejo/runner/v9/act/runner.(*RunContext).isEnabled()
      /.../forgejo-runner/act/runner/run_context.go:1069 +0x3c7
  code.forgejo.org/forgejo/runner/v9/act/runner.(*RunContext).Executor.func1()
      /.../forgejo-runner/act/runner/run_context.go:964 +0x4b
  code.forgejo.org/forgejo/runner/v9/act/runner.(*runnerImpl).NewPlanExecutor.func1.1()
      /.../forgejo-runner/act/runner/runner.go:223 +0x271
  code.forgejo.org/forgejo/runner/v9/act/runner.(*runnerImpl).NewPlanExecutor.func1.NewParallelExecutor.2.1()
      /.../forgejo-runner/act/common/executor.go:107 +0x61
  code.forgejo.org/forgejo/runner/v9/act/runner.(*runnerImpl).NewPlanExecutor.func1.NewParallelExecutor.2.gowrap1()
      /.../forgejo-runner/act/common/executor.go:109 +0x4f

Previous read at 0x00c00047e0b0 by goroutine 1742:
  code.forgejo.org/forgejo/runner/v9/act/model.(*Job).RunsOn()
      /.../forgejo-runner/act/model/workflow.go:361 +0x3c4
  code.forgejo.org/forgejo/runner/v9/act/runner.(*RunContext).runsOnImage()
      /.../forgejo-runner/act/runner/run_context.go:991 +0x57a
  code.forgejo.org/forgejo/runner/v9/act/runner.(*RunContext).platformImage()
      /.../forgejo-runner/act/runner/run_context.go:1032 +0x77
  code.forgejo.org/forgejo/runner/v9/act/runner.(*RunContext).isEnabled()
      /.../forgejo-runner/act/runner/run_context.go:1069 +0x3c7
  code.forgejo.org/forgejo/runner/v9/act/runner.(*RunContext).Executor.func1()
      /.../forgejo-runner/act/runner/run_context.go:964 +0x4b
  code.forgejo.org/forgejo/runner/v9/act/runner.(*runnerImpl).NewPlanExecutor.func1.1()
      /.../forgejo-runner/act/runner/runner.go:223 +0x271
  code.forgejo.org/forgejo/runner/v9/act/runner.(*runnerImpl).NewPlanExecutor.func1.NewParallelExecutor.2.1()
      /.../forgejo-runner/act/common/executor.go:107 +0x61
  code.forgejo.org/forgejo/runner/v9/act/runner.(*runnerImpl).NewPlanExecutor.func1.NewParallelExecutor.2.gowrap1()
      /.../forgejo-runner/act/common/executor.go:109 +0x4f
...
==================
```

<!--start release-notes-assistant-->
<!--URL:https://code.forgejo.org/forgejo/runner-->
- bug fixes
  - [PR](https://code.forgejo.org/forgejo/runner/pulls/871): <!--number 871 --><!--line 0 --><!--description Zml4OiBkYXRhIHJhY2UgaW4gJ3J1bnMtb24nIGV4cHJlc3Npb25zIGNhdXNlcyBpbmNvcnJlY3Qgam9iIGxhYmVscyBkdXJpbmcgZXhlY3V0aW9u-->fix: data race in 'runs-on' expressions causes incorrect job labels during execution<!--description-->
<!--end release-notes-assistant-->

Reviewed-on: https://code.forgejo.org/forgejo/runner/pulls/871
Reviewed-by: earl-warren <earl-warren@noreply.code.forgejo.org>
Co-authored-by: Mathieu Fenniak <mathieu@fenniak.net>
Co-committed-by: Mathieu Fenniak <mathieu@fenniak.net>
2025-08-16 20:44:40 +00:00
..
testdata chore(tests): add coverage for ./pkg/runner (#202) 2025-07-28 06:18:46 +00:00
action.go chore: modernize code (#857) 2025-08-15 04:54:13 +00:00
github_context.go chore: modernize code (#857) 2025-08-15 04:54:13 +00:00
github_context_test.go chore: modernize code (#857) 2025-08-15 04:54:13 +00:00
job_context.go refactor: export and move shared contexts into pkg/model (#931) 2021-12-22 19:52:09 +00:00
planner.go chore: modernize code (#857) 2025-08-15 04:54:13 +00:00
planner_test.go feat!: add the validate argument to reading workflows (#180) 2025-07-16 08:46:36 +00:00
step_result.go refactor: fix savestate in pre steps (#1466) 2022-12-15 17:08:31 +00:00
workflow.go fix: data race in 'runs-on' expressions causes incorrect job labels during execution (#871) 2025-08-16 20:44:40 +00:00
workflow_test.go fix: data race condition causing incorrect shell on a task step if it referenced a matrix variable (#865) 2025-08-15 21:10:53 +00:00