Channel#wait_for_confirms may return before all confirmations for outstanding publishes are received #424

Closed
mitio opened this Issue Jul 18, 2016 · 2 comments

Projects

None yet

2 participants

@mitio
Contributor
mitio commented Jul 18, 2016

I discovered a possible race condition in Channel#wait_for_confirms which can lead to wait_for_confirms returning before RabbitMQ has acknowledged (or nack'ed) the published message. This actually hit us in production.

Suppose the following happens:

  1. We have a channel in "confirm select" mode.
  2. We do a basic_publish to that channel. Some time passes, say >0.5s.
  3. During that time, an ack is received by Bunny and handled in handle_ack_or_nack. @unconfirmed_set is emptied, true is pushed to @confirms_continuations.
  4. We do a second baslic_publish to that same channel.
  5. Immediately afterwards we issue wait_for_confirms to wait for all confirmations. It sees that @unconfirmed_set has something in it and issues @confirms_continuations.poll but that returns immediately and unexpectedly because of the true value pushed to it at step 3. We haven't received an ack or nack for the second publish yet.

The way I understand the documentation is that doing a bunch of publishes and then calling wait_for_confirms is a completely valid use case, but unless I'm missing something obvious, it's pretty evident from the code that each ack that comes will result in another true value pushed to @confirms_continuations and that many wait_for_confirms will immediately succeed afterwards, no matter how many messages are in @unconfirmed_set.

In our code we actually do a publish and wait_for_confirms immediately after that, but the race condition can still occur (and it did).

I've also written a small script that demonstrates the issue.

Tested on: Bunny 2.4.0 and Ruby 2.1.2 on macOS

@michaelklishin michaelklishin added the bug label Jul 18, 2016
@michaelklishin
Member

OK, thanks, that sounds like a legit issue.

@michaelklishin
Member

@mitio thank you, out in 2.5.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment