Skip to content

Commit

Permalink
Avoid race between multi-message ack and publish
Browse files Browse the repository at this point in the history
PR ruby-amqp#617 introduced an optimization for multi-message acks but failed
to protect @unconfirmed_set. In edge cases #basic_publish would
attempt to modify it during iteration with Enumerable#min:

    <ruby>/lib/ruby/3.1.0/set.rb:522:in `add': can't add a new key
    into hash during iteration (RuntimeError)
    from <bunny>/lib/bunny/channel.rb:555:in `block in basic_publish'
    from <bunny>/lib/bunny/channel.rb:554:in `synchronize'
    from <bunny>/lib/bunny/channel.rb:554:in `basic_publish'
    from <bunny>/lib/bunny/exchange.rb:141:in `publish'
  • Loading branch information
romuloceccon committed Jul 13, 2022
1 parent 138a8bf commit e7f8feb
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions lib/bunny/channel.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1788,12 +1788,12 @@ def handle_basic_return(basic_return, properties, content)

# @private
def handle_ack_or_nack(delivery_tag_before_offset, multiple, nack)
delivery_tag = delivery_tag_before_offset + @delivery_tag_offset
confirmed_range_start = multiple ? @delivery_tag_offset + @unconfirmed_set.min : delivery_tag
confirmed_range_end = delivery_tag
confirmed_range = (confirmed_range_start..confirmed_range_end)

@unconfirmed_set_mutex.synchronize do
delivery_tag = delivery_tag_before_offset + @delivery_tag_offset
confirmed_range_start = multiple ? @delivery_tag_offset + @unconfirmed_set.min : delivery_tag
confirmed_range_end = delivery_tag
confirmed_range = (confirmed_range_start..confirmed_range_end)

if nack
@nacked_set.merge(@unconfirmed_set & confirmed_range)
end
Expand Down

0 comments on commit e7f8feb

Please sign in to comment.