Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Data race in actionlint when using configuration file #333

Closed
hansgylling opened this issue Aug 4, 2023 · 3 comments
Closed

Data race in actionlint when using configuration file #333

hansgylling opened this issue Aug 4, 2023 · 3 comments
Labels
bug Something isn't working

Comments

@hansgylling
Copy link

hansgylling commented Aug 4, 2023

I recently discovered this tool and it seems very useful. As I usually do with Go commands, I installed from source with the race detector enabled. When I tried actionlint I could trigger a data race every time when using a configuration file, but I couldn't trigger the race when there was no configuration file.

Here's a session of commands of how to trigger the data race. The actual path to my repo is replaced.

$ go install -trimpath -race github.com/rhysd/actionlint/cmd/actionlint@latest
$ actionlint -version
v1.6.25
installed by building from source
built with go1.19.12 compiler for linux/amd64
$ actionlint -init-config
Config file was generated at "$PATH_TO_MY_REPO/.github/actionlint.yaml"
$ cat .github/actionlint.yaml
self-hosted-runner:
  # Labels of self-hosted runner in array of strings.
  labels: []
# Configuration variables in array of strings defined in your repository or
# organization. `null` means disabling configuration variables check.
# Empty array means no configuration variable is allowed.
config-variables: null
$ actionlint
==================
WARNING: DATA RACE
Write at 0x00c0001de5e0 by goroutine 43:
  github.com/rhysd/actionlint.(*Project).LoadConfig()
      github.com/rhysd/actionlint@v1.6.25/project.go:78 +0x21d
  github.com/rhysd/actionlint.(*Linter).check()
      github.com/rhysd/actionlint@v1.6.25/linter.go:469 +0x213
  github.com/rhysd/actionlint.(*Linter).LintFiles.func1()
      github.com/rhysd/actionlint@v1.6.25/linter.go:335 +0x245
  golang.org/x/sync/errgroup.(*Group).Go.func1()
      golang.org/x/sync@v0.2.0/errgroup/errgroup.go:75 +0x86

Previous read at 0x00c0001de5e0 by goroutine 27:
  github.com/rhysd/actionlint.(*Project).LoadConfig()
      github.com/rhysd/actionlint@v1.6.25/project.go:64 +0x55
  github.com/rhysd/actionlint.(*Linter).check()
      github.com/rhysd/actionlint@v1.6.25/linter.go:469 +0x213
  github.com/rhysd/actionlint.(*Linter).LintFiles.func1()
      github.com/rhysd/actionlint@v1.6.25/linter.go:335 +0x245
  golang.org/x/sync/errgroup.(*Group).Go.func1()
      golang.org/x/sync@v0.2.0/errgroup/errgroup.go:75 +0x86

Goroutine 43 (running) created at:
  golang.org/x/sync/errgroup.(*Group).Go()
      golang.org/x/sync@v0.2.0/errgroup/errgroup.go:72 +0x12e
  github.com/rhysd/actionlint.(*Linter).LintFiles()
      github.com/rhysd/actionlint@v1.6.25/linter.go:321 +0x78b
  github.com/rhysd/actionlint.(*Linter).LintDir()
      github.com/rhysd/actionlint@v1.6.25/linter.go:272 +0x22b
  github.com/rhysd/actionlint.(*Linter).LintRepository()
      github.com/rhysd/actionlint@v1.6.25/linter.go:243 +0x204
  github.com/rhysd/actionlint.(*Command).runLinter()
      github.com/rhysd/actionlint@v1.6.25/command.go:91 +0x387
  github.com/rhysd/actionlint.(*Command).Main()
      github.com/rhysd/actionlint@v1.6.25/command.go:180 +0xcf9
  main.main()
      github.com/rhysd/actionlint@v1.6.25/cmd/actionlint/main.go:15 +0x12f

Goroutine 27 (running) created at:
  golang.org/x/sync/errgroup.(*Group).Go()
      golang.org/x/sync@v0.2.0/errgroup/errgroup.go:72 +0x12e
  github.com/rhysd/actionlint.(*Linter).LintFiles()
      github.com/rhysd/actionlint@v1.6.25/linter.go:321 +0x78b
  github.com/rhysd/actionlint.(*Linter).LintDir()
      github.com/rhysd/actionlint@v1.6.25/linter.go:272 +0x22b
  github.com/rhysd/actionlint.(*Linter).LintRepository()
      github.com/rhysd/actionlint@v1.6.25/linter.go:243 +0x204
  github.com/rhysd/actionlint.(*Command).runLinter()
      github.com/rhysd/actionlint@v1.6.25/command.go:91 +0x387
  github.com/rhysd/actionlint.(*Command).Main()
      github.com/rhysd/actionlint@v1.6.25/command.go:180 +0xcf9
  main.main()
      github.com/rhysd/actionlint@v1.6.25/cmd/actionlint/main.go:15 +0x12f
==================
@rhysd rhysd added the bug Something isn't working label Aug 5, 2023
@rhysd
Copy link
Owner

rhysd commented Aug 5, 2023

Thank you so much for catching this. Actually LoadConfig is not thread safe but it is called by multiple Goroutines. I could reproduce this issue on my local.

@rhysd
Copy link
Owner

rhysd commented Aug 5, 2023

I think I should run actionlint for actionlint repository itself enabling a race detector. I'll configure CI after fixing this issue.

@rhysd rhysd closed this as completed in 43f7b04 Aug 5, 2023
rhysd added a commit that referenced this issue Aug 5, 2023
@hansgylling
Copy link
Author

It would be nice with a new patch release now that this bug is fixed. Then I could experiment with actionlint with a stable release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants