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 rule function-length #736

Closed
hansgylling opened this issue Aug 18, 2022 · 7 comments · Fixed by #745
Closed

Data race in rule function-length #736

hansgylling opened this issue Aug 18, 2022 · 7 comments · Fixed by #745
Assignees
Labels

Comments

@hansgylling
Copy link

hansgylling commented Aug 18, 2022

Describe the bug
When revive is installed with Go's race detector enabled, any use of the function-length rule triggers a data race.

To Reproduce
Steps to reproduce the behavior:

  1. Install revive with the race detector enabled go install -a -race github.com/mgechev/revive@latest
  2. Run it with the following flags & configuration file:
# shell command
revive -config revive_function_length_data_race.toml ./...
# config file
[rule.function-length]
  arguments = [10,0]

Expected behavior
No data race detected.

Logs

==================
WARNING: DATA RACE
Write at 0x00c0000fa210 by goroutine 175:
  github.com/mgechev/revive/rule.(*FunctionLength).configure()
      /home/hansg/go/pkg/mod/github.com/mgechev/revive@v1.2.3/rule/function-length.go:23 +0x98
  github.com/mgechev/revive/rule.(*FunctionLength).Apply()
      /home/hansg/go/pkg/mod/github.com/mgechev/revive@v1.2.3/rule/function-length.go:31 +0x84
  github.com/mgechev/revive/lint.(*File).lint()
      /home/hansg/go/pkg/mod/github.com/mgechev/revive@v1.2.3/lint/file.go:105 +0x201
  github.com/mgechev/revive/lint.(*Package).lint.func1()
      /home/hansg/go/pkg/mod/github.com/mgechev/revive@v1.2.3/lint/package.go:185 +0x104
  github.com/mgechev/revive/lint.(*Package).lint.func2()
      /home/hansg/go/pkg/mod/github.com/mgechev/revive@v1.2.3/lint/package.go:187 +0x47

Previous read at 0x00c0000fa210 by goroutine 174:
  github.com/mgechev/revive/rule.(*FunctionLength).Apply()
      /home/hansg/go/pkg/mod/github.com/mgechev/revive@v1.2.3/rule/function-length.go:37 +0xcc
  github.com/mgechev/revive/lint.(*File).lint()
      /home/hansg/go/pkg/mod/github.com/mgechev/revive@v1.2.3/lint/file.go:105 +0x201
  github.com/mgechev/revive/lint.(*Package).lint.func1()
      /home/hansg/go/pkg/mod/github.com/mgechev/revive@v1.2.3/lint/package.go:185 +0x104
  github.com/mgechev/revive/lint.(*Package).lint.func2()
      /home/hansg/go/pkg/mod/github.com/mgechev/revive@v1.2.3/lint/package.go:187 +0x47

Goroutine 175 (running) created at:
  github.com/mgechev/revive/lint.(*Package).lint()
      /home/hansg/go/pkg/mod/github.com/mgechev/revive@v1.2.3/lint/package.go:184 +0xed
  github.com/mgechev/revive/lint.(*Linter).lintPackage()
      /home/hansg/go/pkg/mod/github.com/mgechev/revive@v1.2.3/lint/linter.go:106 +0x2b7
  github.com/mgechev/revive/lint.(*Linter).Lint.func1()
      /home/hansg/go/pkg/mod/github.com/mgechev/revive@v1.2.3/lint/linter.go:64 +0x137
  github.com/mgechev/revive/lint.(*Linter).Lint.func3()
      /home/hansg/go/pkg/mod/github.com/mgechev/revive@v1.2.3/lint/linter.go:69 +0x66

Goroutine 174 (finished) created at:
  github.com/mgechev/revive/lint.(*Package).lint()
      /home/hansg/go/pkg/mod/github.com/mgechev/revive@v1.2.3/lint/package.go:184 +0xed
  github.com/mgechev/revive/lint.(*Linter).lintPackage()
      /home/hansg/go/pkg/mod/github.com/mgechev/revive@v1.2.3/lint/linter.go:106 +0x2b7
  github.com/mgechev/revive/lint.(*Linter).Lint.func1()
      /home/hansg/go/pkg/mod/github.com/mgechev/revive@v1.2.3/lint/linter.go:64 +0x137
  github.com/mgechev/revive/lint.(*Linter).Lint.func3()
      /home/hansg/go/pkg/mod/github.com/mgechev/revive@v1.2.3/lint/linter.go:69 +0x66
==================

Desktop (please complete the following information):

  • OS: Debian 9 (stretch)
  • Version of Go: Go 1.19 (but also reproduced with Go 1.18 and 1.17)

Additional context
N/A

@chavacava chavacava self-assigned this Aug 18, 2022
@chavacava chavacava added the bug label Aug 18, 2022
@chavacava
Copy link
Collaborator

Hi @hansgylling, thanks for filling the issue.
Could you please check if the branch https://github.com/mgechev/revive/tree/fix/736 fixes the problem?

@hansgylling
Copy link
Author

Yes, I can test. I can probably do it tomorrow or on Tuesday.

@hansgylling
Copy link
Author

I have tested now and I can't reproduce the race condition using your branch. I tried both the original config file in the bug report and a config file using all rules at once.

@chavacava
Copy link
Collaborator

Thanks for the check @hansgylling

@mgechev
Copy link
Owner

mgechev commented Aug 29, 2022

@chavacava let me know if you'd like to open a PR so we can do a release.

@chavacava
Copy link
Collaborator

I have restricted Internet access until the week-end. If the release can wait, I'll make the PR by saturday

chavacava added a commit that referenced this issue Sep 3, 2022
chavacava added a commit that referenced this issue Sep 10, 2022
@hansgylling
Copy link
Author

Thanks for fixing this bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants