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

Possible data race #34

Closed
rzajac opened this issue Jul 19, 2021 · 7 comments
Closed

Possible data race #34

rzajac opened this issue Jul 19, 2021 · 7 comments

Comments

@rzajac
Copy link

rzajac commented Jul 19, 2021

After updating to the version I get:

  ==================
  WARNING: DATA RACE
  Write at 0x00c000474220 by goroutine 33:
    github.com/wagslane/go-rabbitmq.(*Publisher).startNotifyFlowHandler()
        go/pkg/mod/github.com/wagslane/go-rabbitmq@v0.6.2/publish.go:234 +0x70

  Previous read at 0x00c000474220 by goroutine 24:
    github.com/wagslane/go-rabbitmq.NewPublisher()
        go/pkg/mod/github.com/wagslane/go-rabbitmq@v0.6.2/publish.go:167 +0x3b3
    git.vonroll-infratec.com/go/meas/pkg/queue.NewPublisher()
        ws/go/meas/pkg/queue/rab_pub.go:48 +0x604
    git.vonroll-infratec.com/go/meas/pkg/queue.(*RabbitSuite).SetupTest()
        ws/go/meas/pkg/queue/rabbit_test.go:43 +0x119
    github.com/stretchr/testify/suite.Run.func1()
        go/pkg/mod/github.com/rzajac/testify@v1.6.2-0.20200929081604-1a836a85454e/suite/suite.go:148 +0x8b4
    testing.tRunner()
        /usr/local/go/src/testing/testing.go:1193 +0x202

  Goroutine 33 (running) created at:
    github.com/wagslane/go-rabbitmq.(*Publisher).startNotifyHandlers()
        go/pkg/mod/github.com/wagslane/go-rabbitmq@v0.6.2/publish.go:229 +0x164
    github.com/wagslane/go-rabbitmq.NewPublisher.func1()
        go/pkg/mod/github.com/wagslane/go-rabbitmq@v0.6.2/publish.go:160 +0x44

  Goroutine 24 (running) created at:
    testing.(*T).Run()
        /usr/local/go/src/testing/testing.go:1238 +0x5d7
    github.com/stretchr/testify/suite.runTests()
        go/pkg/mod/github.com/rzajac/testify@v1.6.2-0.20200929081604-1a836a85454e/suite/suite.go:203 +0xf7
    github.com/stretchr/testify/suite.Run()
        go/pkg/mod/github.com/rzajac/testify@v1.6.2-0.20200929081604-1a836a85454e/suite/suite.go:176 +0x944
    git.vonroll-infratec.com/go/meas/pkg/queue.TestRabbit()
        ws/go/meas/pkg/queue/rabbit_test.go:19 +0xa4
    testing.tRunner()
        /usr/local/go/src/testing/testing.go:1193 +0x202
  ==================

in my tests.

@rzajac
Copy link
Author

rzajac commented Jul 20, 2021

OK so I run all my tests with race detector. I was able to fix race errors by returning pointer to the Publisher:

func NewPublisher(url string, config amqp.Config, optionFuncs ...func(*PublisherOptions)) (Publisher, <-chan Return, error) {

@wagslane
Copy link
Owner

@rzajac Would you mind opening a PR with the fix you described (assuming its the right way to fix this)

@rzajac
Copy link
Author

rzajac commented Jul 20, 2021

@wagslane Well the fix was just a first thought. Not sure if this is the right way to do it. I can do the PR.

rzajac added a commit to rzajac/go-rabbitmq that referenced this issue Jul 20, 2021
rzajac added a commit to rzajac/go-rabbitmq that referenced this issue Jul 20, 2021
@cyberbeast
Copy link

Has this been fixed? I am also seeing the data race.

@wagslane
Copy link
Owner

If someone could add a test to the test suite that demonstrates the race that would help this move along. Otherwise, might be a second before I can get to it.

@pete-woods
Copy link

From a quick skim through the code, it looks like you write to the various channel handles inside mutex locks, but read from them without locking in the publishing codepath.

@wagslane
Copy link
Owner

wagslane commented Dec 7, 2021

@rzajac Can you try the latest? Should be fixed

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

No branches or pull requests

4 participants