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

Fix a data race in getAttrInfo #204

Merged
merged 2 commits into from
Nov 8, 2023
Merged

Fix a data race in getAttrInfo #204

merged 2 commits into from
Nov 8, 2023

Conversation

kalmant
Copy link
Contributor

@kalmant kalmant commented Nov 3, 2023

Please ensure all items are complete before opening.

  • Tick to sign-off your agreement to the Developer's Certificate of Origin
  • You have added tests for any code changes - test needs to be run with go test -race
  • You have updated the CHANGES.md - CHANGELOG.md should probably only get updated once there's a new version that includes this fix.
  • You have completed the PR template below:

What

Fixes an issue raised in #203

How

By using sync.Once instead of a bool to ensure the initialization block gets called only once.

Testing

Running go test -race ./... after I added TestGetAttrInfoConcurrentCalls without the fix would yield output such as:

==================
WARNING: DATA RACE
Read at 0x0001058dcc8d by goroutine 21:
  github.com/ibm-messaging/mq-golang/v5/ibmmq.getAttrInfo()
      /Users/foo/bar/mq-golang/ibmmq/mqiattrs.go:144 +0x34
  github.com/ibm-messaging/mq-golang/v5/ibmmq.TestGetAttrInfoConcurrentCalls.func1()
      /Users/foo/bar/mq-golang/ibmmq/ibmmq_test.go:335 +0x4c

Previous write at 0x0001058dcc8d by goroutine 15:
  github.com/ibm-messaging/mq-golang/v5/ibmmq.getAttrInfo()
      /Users/foo/bar/mq-golang/ibmmq/mqiattrs.go:158 +0x1b8
  github.com/ibm-messaging/mq-golang/v5/ibmmq.TestGetAttrInfoConcurrentCalls.func1()
      /Users/foo/bar/mq-golang/ibmmq/ibmmq_test.go:335 +0x4c

Goroutine 21 (running) created at:
  github.com/ibm-messaging/mq-golang/v5/ibmmq.TestGetAttrInfoConcurrentCalls()
      /Users/foo/bar/mq-golang/ibmmq/ibmmq_test.go:334 +0x74
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.21.3/libexec/src/testing/testing.go:1595 +0x194
  testing.(*T).Run.func1()
      /opt/homebrew/Cellar/go/1.21.3/libexec/src/testing/testing.go:1648 +0x40

Goroutine 15 (finished) created at:
  github.com/ibm-messaging/mq-golang/v5/ibmmq.TestGetAttrInfoConcurrentCalls()
      /Users/foo/bar/mq-golang/ibmmq/ibmmq_test.go:334 +0x74
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.21.3/libexec/src/testing/testing.go:1595 +0x194
  testing.(*T).Run.func1()
      /opt/homebrew/Cellar/go/1.21.3/libexec/src/testing/testing.go:1648 +0x40
==================
==================
WARNING: DATA RACE
Read at 0x00c00009e360 by goroutine 18:
  runtime.mapaccess1_fast32()
      /opt/homebrew/Cellar/go/1.21.3/libexec/src/runtime/map_fast32.go:13 +0x1bc
  github.com/ibm-messaging/mq-golang/v5/ibmmq.getAttrInfo()
      /Users/foo/bar/mq-golang/ibmmq/mqiattrs.go:162 +0x254
  github.com/ibm-messaging/mq-golang/v5/ibmmq.TestGetAttrInfoConcurrentCalls.func1()
      /Users/foo/bar/mq-golang/ibmmq/ibmmq_test.go:335 +0x4c

Previous write at 0x00c00009e360 by goroutine 15:
  runtime.mapaccess2_fast32()
      /opt/homebrew/Cellar/go/1.21.3/libexec/src/runtime/map_fast32.go:53 +0x1cc
  github.com/ibm-messaging/mq-golang/v5/ibmmq.getAttrInfo()
      /Users/foo/bar/mq-golang/ibmmq/mqiattrs.go:156 +0x164
  github.com/ibm-messaging/mq-golang/v5/ibmmq.TestGetAttrInfoConcurrentCalls.func1()
      /Users/foo/bar/mq-golang/ibmmq/ibmmq_test.go:335 +0x4c

Goroutine 18 (running) created at:
  github.com/ibm-messaging/mq-golang/v5/ibmmq.TestGetAttrInfoConcurrentCalls()
      /Users/foo/bar/mq-golang/ibmmq/ibmmq_test.go:334 +0x74
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.21.3/libexec/src/testing/testing.go:1595 +0x194
  testing.(*T).Run.func1()
      /opt/homebrew/Cellar/go/1.21.3/libexec/src/testing/testing.go:1648 +0x40

Goroutine 15 (finished) created at:
  github.com/ibm-messaging/mq-golang/v5/ibmmq.TestGetAttrInfoConcurrentCalls()
      /Users/foo/bar/mq-golang/ibmmq/ibmmq_test.go:334 +0x74
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.21.3/libexec/src/testing/testing.go:1595 +0x194
  testing.(*T).Run.func1()
      /opt/homebrew/Cellar/go/1.21.3/libexec/src/testing/testing.go:1648 +0x40
==================
--- FAIL: TestGetAttrInfoConcurrentCalls (0.01s)
    testing.go:1465: race detected during execution of test

Now it's ok github.com/ibm-messaging/mq-golang/v5/ibmmq 1.453s.

The test does not check whether getAttrInfo's overall behavior stayed the same, it only checks for data races!

Issues

#203

@raja
Copy link

raja commented Nov 6, 2023

@ibmmqmet How are pull requests reviewed/assigned?

@ibmmqmet ibmmqmet merged commit 7c3d21e into ibm-messaging:master Nov 8, 2023
@ibmmqmet
Copy link
Collaborator

ibmmqmet commented Nov 8, 2023

Thanks for finding that. It's now been merged and pushed to a new version

@kalmant
Copy link
Contributor Author

kalmant commented Nov 9, 2023

Thank you!

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

Successfully merging this pull request may close these issues.

3 participants