wagslane / go-rabbitmq

A wrapper of streadway/amqp that provides reconnection logic and sane defaults
https://blog.boot.dev/golang/connecting-to-rabbitmq-in-golang-easy/
MIT License
785 stars 128 forks source link

Race condition when calling PublishWithDeferredConfirmWithContext #130

Closed buni closed 1 year ago

buni commented 1 year ago

There is a race condition inside this function, which is caused by this write.

The issue can be reproduced by running go test -v -race in this branch.

The fix itself is pretty simple, although I'm not sure if it introduces any other issues (deadlocks?) since I'm not very familiar with the package internals.

==================
WARNING: DATA RACE
Read at 0x00c0002de0c8 by goroutine 65:
  github.com/rabbitmq/amqp091-go.(*Channel).PublishWithDeferredConfirmWithContext()
      /home/go-rabbitmq/vendor/github.com/rabbitmq/amqp091-go/channel.go:1463 +0x895
  github.com/wagslane/go-rabbitmq/internal/channelmanager.(*ChannelManager).PublishWithDeferredConfirmWithContextSafe()
      /home/go-rabbitmq/internal/channelmanager/safe_wraps.go:180 +0x1b9
  github.com/wagslane/go-rabbitmq.(*Publisher).PublishWithDeferredConfirmWithContext()
      /home/go-rabbitmq/publish.go:259 +0x964
  github.com/wagslane/go-rabbitmq/tests_test.TestPublisherConfirmRaceCondition()
      /home/go-rabbitmq/tests/publisher_test.go:39 +0x552
  testing.tRunner()
      /snap/go/current/src/testing/testing.go:1576 +0x216
  testing.(*T).Run.func1()
      /snap/go/current/src/testing/testing.go:1629 +0x47

Previous write at 0x00c0002de0c8 by goroutine 76:
  github.com/rabbitmq/amqp091-go.(*Channel).Confirm()
      /home/go-rabbitmq/vendor/github.com/rabbitmq/amqp091-go/channel.go:1613 +0xe4
  github.com/wagslane/go-rabbitmq/internal/channelmanager.(*ChannelManager).ConfirmSafe()
      /home/go-rabbitmq/internal/channelmanager/safe_wraps.go:209 +0xed
  github.com/wagslane/go-rabbitmq.(*Publisher).startPublishHandler()
      /home/go-rabbitmq/publish.go:342 +0xb7
  github.com/wagslane/go-rabbitmq.(*Publisher).NotifyPublish.func1()
      /home/go-rabbitmq/publish.go:316 +0x39

Goroutine 65 (running) created at:
  testing.(*T).Run()
      /snap/go/current/src/testing/testing.go:1629 +0x805
  testing.runTests.func1()
      /snap/go/current/src/testing/testing.go:2036 +0x8d
  testing.tRunner()
      /snap/go/current/src/testing/testing.go:1576 +0x216
  testing.runTests()
      /snap/go/current/src/testing/testing.go:2034 +0x87c
  testing.(*M).Run()
      /snap/go/current/src/testing/testing.go:1906 +0xb44
  github.com/wagslane/go-rabbitmq/tests_test.TestMain()
      /home/go-rabbitmq/tests/main_test.go:12 +0x38
  main.main()
      _testmain.go:49 +0x324

Goroutine 76 (running) created at:
  github.com/wagslane/go-rabbitmq.(*Publisher).NotifyPublish()
      /home/go-rabbitmq/publish.go:316 +0x136
  github.com/wagslane/go-rabbitmq.NewPublisher()
      /home/go-rabbitmq/publish.go:122 +0x749
  github.com/wagslane/go-rabbitmq/tests_test.TestPublisherConfirmRaceCondition()
      /home/go-rabbitmq/tests/publisher_test.go:25 +0x304
  testing.tRunner()
      /snap/go/current/src/testing/testing.go:1576 +0x216
  testing.(*T).Run.func1()
      /snap/go/current/src/testing/testing.go:1629 +0x47
==================
wagslane commented 1 year ago

Fix merged!