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.

Sign up for GitHub

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

Jump to bottom

Fix data race in hooks.test package #1362

Merged
merged 1 commit into from
Jan 7, 2023
Merged

Fix data race in hooks.test package #1362

merged 1 commit into from
Jan 7, 2023

Conversation

FrancoisWagner
Copy link

Prior fix:

go test -race -timeout 30s -run ^TestNewLocal$ github.com/sirupsen/logrus/hooks/test
time="2022-12-22T10:57:57+01:00" level=info msg=info
time="2022-12-22T10:57:57+01:00" level=info msg=info
time="2022-12-22T10:57:57+01:00" level=info msg=info
==================
WARNING: DATA RACE
Write at 0x00c00009f4d0 by goroutine 7:
  runtime.mapaccess2_fast32()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/runtime/map_fast32.go:53 +0x1cc
  github.com/sirupsen/logrus.LevelHooks.Add()
      /Users/wfrancoi/Downloads/logrus/hooks.go:20 +0x170
  github.com/sirupsen/logrus/hooks/test.NewLocal()
      /Users/wfrancoi/Downloads/logrus/hooks/test/test.go:35 +0x38
  github.com/sirupsen/logrus/hooks/test.TestNewLocal()
      /Users/wfrancoi/Downloads/logrus/hooks/test/test_test.go:102 +0x3b8
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1439 +0x18c
  testing.(*T).Run.func1()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1486 +0x44

Previous read at 0x00c00009f4d0 by goroutine 8:
  github.com/sirupsen/logrus.(*Entry).fireHooks()
      /Users/wfrancoi/Downloads/logrus/entry.go:274 +0x98
  github.com/sirupsen/logrus.(*Entry).log()
      /Users/wfrancoi/Downloads/logrus/entry.go:242 +0x624
  github.com/sirupsen/logrus.(*Entry).Log()
      /Users/wfrancoi/Downloads/logrus/entry.go:304 +0x84
time="2022-12-22T10:57:57+01:00" level=info msg=info
  github.com/sirupsen/logrus.(*Logger).Log()
      /Users/wfrancoi/Downloads/logrus/logger.go:204 +0x70
  github.com/sirupsen/logrus.(*Logger).Info()
      /Users/wfrancoi/Downloads/logrus/logger.go:226 +0x64
  github.com/sirupsen/logrus/hooks/test.TestNewLocal.func1()
      /Users/wfrancoi/Downloads/logrus/hooks/test/test_test.go:97 +0x34
  github.com/sirupsen/logrus/hooks/test.TestNewLocal.func3()
      /Users/wfrancoi/Downloads/logrus/hooks/test/test_test.go:99 +0x48

Goroutine 7 (running) created at:
  testing.(*T).Run()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1486 +0x560
  testing.runTests.func1()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1839 +0x94
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1439 +0x18c
  testing.runTests()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1837 +0x6c8
  testing.(*M).Run()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1719 +0x878
  main.main()
      _testmain.go:53 +0x2fc

Goroutine 8 (running) created at:
  github.com/sirupsen/logrus/hooks/test.TestNewLocal()
      /Users/wfrancoi/Downloads/logrus/hooks/test/test_test.go:96 +0x2a0
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1439 +0x18c
  testing.(*T).Run.func1()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1486 +0x44
==================
time="2022-12-22T10:57:57+01:00" level=info msg=info
==================
WARNING: DATA RACE
Read at 0x00c0001340a0 by goroutine 13:
  github.com/sirupsen/logrus.(*Entry).fireHooks()
      /Users/wfrancoi/Downloads/logrus/entry.go:275 +0x170
  github.com/sirupsen/logrus.(*Entry).log()
      /Users/wfrancoi/Downloads/logrus/entry.go:242 +0x624
  github.com/sirupsen/logrus.(*Entry).Log()
      /Users/wfrancoi/Downloads/logrus/entry.go:304 +0x84
  github.com/sirupsen/logrus.(*Logger).Log()
      /Users/wfrancoi/Downloads/logrus/logger.go:204 +0x70
  github.com/sirupsen/logrus.(*Logger).Info()
      /Users/wfrancoi/Downloads/logrus/logger.go:226 +0x64
  github.com/sirupsen/logrus/hooks/test.TestNewLocal.func1()
      /Users/wfrancoi/Downloads/logrus/hooks/test/test_test.go:97 +0x34
  github.com/sirupsen/logrus/hooks/test.TestNewLocal.func3()
      /Users/wfrancoi/Downloads/logrus/hooks/test/test_test.go:99 +0x48

Previous write at 0x00c0001340a0 by goroutine 7:
  github.com/sirupsen/logrus.LevelHooks.Add()
      /Users/wfrancoi/Downloads/logrus/hooks.go:20 +0x17c
  github.com/sirupsen/logrus/hooks/test.NewLocal()
      /Users/wfrancoi/Downloads/logrus/hooks/test/test.go:35 +0x38
  github.com/sirupsen/logrus/hooks/test.TestNewLocal()
      /Users/wfrancoi/Downloads/logrus/hooks/test/test_test.go:102 +0x3b8
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1439 +0x18c
  testing.(*T).Run.func1()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1486 +0x44

Goroutine 13 (running) created at:
  github.com/sirupsen/logrus/hooks/test.TestNewLocal()
      /Users/wfrancoi/Downloads/logrus/hooks/test/test_test.go:96 +0x2a0
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1439 +0x18c
  testing.(*T).Run.func1()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1486 +0x44

Goroutine 7 (running) created at:
  testing.(*T).Run()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1486 +0x560
  testing.runTests.func1()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1839 +0x94
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1439 +0x18c
  testing.runTests()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1837 +0x6c8
  testing.(*M).Run()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1719 +0x878
  main.main()
      _testmain.go:53 +0x2fc
==================
==================
WARNING: DATA RACE
Read at 0x00c00009b0c0 by goroutine 13:
  github.com/sirupsen/logrus.LevelHooks.Fire()
      /Users/wfrancoi/Downloads/logrus/hooks.go:27 +0x9c
  github.com/sirupsen/logrus.(*Entry).fireHooks()
      /Users/wfrancoi/Downloads/logrus/entry.go:280 +0x238
  github.com/sirupsen/logrus.(*Entry).log()
      /Users/wfrancoi/Downloads/logrus/entry.go:242 +0x624
  github.com/sirupsen/logrus.(*Entry).Log()
      /Users/wfrancoi/Downloads/logrus/entry.go:304 +0x84
  github.com/sirupsen/logrus.(*Logger).Log()
      /Users/wfrancoi/Downloads/logrus/logger.go:204 +0x70
  github.com/sirupsen/logrus.(*Logger).Info()
      /Users/wfrancoi/Downloads/logrus/logger.go:226 +0x64
  github.com/sirupsen/logrus/hooks/test.TestNewLocal.func1()
      /Users/wfrancoi/Downloads/logrus/hooks/test/test_test.go:97 +0x34
  github.com/sirupsen/logrus/hooks/test.TestNewLocal.func3()
      /Users/wfrancoi/Downloads/logrus/hooks/test/test_test.go:99 +0x48

Previous write at 0x00c00009b0c0 by goroutine 7:
  github.com/sirupsen/logrus.LevelHooks.Add()
      /Users/wfrancoi/Downloads/logrus/hooks.go:20 +0x110
  github.com/sirupsen/logrus/hooks/test.NewLocal()
      /Users/wfrancoi/Downloads/logrus/hooks/test/test.go:35 +0x38
time="2022-12-22T10:57:57+01:00" level=info msg=info
  github.com/sirupsen/logrus/hooks/test.TestNewLocal()
      /Users/wfrancoi/Downloads/logrus/hooks/test/test_test.go:102 +0x3b8
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1439 +0x18c
  testing.(*T).Run.func1()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1486 +0x44

Goroutine 13 (running) created at:
  github.com/sirupsen/logrus/hooks/test.TestNewLocal()
      /Users/wfrancoi/Downloads/logrus/hooks/test/test_test.go:96 +0x2a0
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1439 +0x18c
  testing.(*T).Run.func1()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1486 +0x44

Goroutine 7 (running) created at:
  testing.(*T).Run()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1486 +0x560
  testing.runTests.func1()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1839 +0x94
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1439 +0x18c
  testing.runTests()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1837 +0x6c8
  testing.(*M).Run()
      /opt/homebrew/Cellar/go/1.18.4/libexec/src/testing/testing.go:1719 +0x878
  main.main()
      _testmain.go:53 +0x2fc
==================
time="2022-12-22T10:57:57+01:00" level=info msg=info
time="2022-12-22T10:57:57+01:00" level=info msg=info
time="2022-12-22T10:57:57+01:00" level=info msg=info
time="2022-12-22T10:57:57+01:00" level=info msg=info
--- FAIL: TestNewLocal (0.00s)
    testing.go:1312: race detected during execution of test
FAIL
FAIL    github.com/sirupsen/logrus/hooks/test   0.274s
FAIL

After fix:

go test -race -timeout 30s -run ^TestNewLocal$ github.com/sirupsen/logrus/hooks/test
ok      github.com/sirupsen/logrus/hooks/test   0.240s

@FrancoisWagner
Copy link
Author

FrancoisWagner commented Jan 6, 2023 •
edited
Loading

@sirupsen @dgsb could you have a look at this PR?

@sirupsen sirupsen merged commit a448f82 into sirupsen:master Jan 7, 2023
@FrancoisWagner
Copy link
Author

Thanks @sirupsen! Any chance we could have a new patch release with this fix? Thanks!

@sirupsen
Copy link
Owner

sirupsen commented Jan 15, 2023 •
edited
Loading

v1.8.2 now has this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@FrancoisWagner @sirupsen