Skip to content

Commit

Permalink
Fix function_clause
Browse files Browse the repository at this point in the history
Fixes #12398

To repro this crash:
1. Start RabbitMQ v3.13.7 with feature flag message_containers disabled:
```
make run-broker TEST_TMPDIR="$HOME/scratch/rabbit/test" RABBITMQ_FEATURE_FLAGS=quorum_queue,implicit_default_bindings,virtual_host_metadata,maintenance_mode_status,user_limits,feature_flags_v2,stream_queue,classic_queue_type_delivery_support,classic_mirrored_queue_version,stream_single_active_consumer,direct_exchange_routing_v2,listener_records_in_ets,tracking_records_in_ets
```
In the Management UI
2. Create a quorum queue with x-delivery-limit=10
3. Publish a message to this queue.
4. Requeue this message two times.
5. ./sbin/rabbitmqctl enable_feature_flag all
6. Stop the node
7. git checkout v4.0.2
8. make run-broker TEST_TMPDIR="$HOME/scratch/rabbit/test"
9. Again in the Management UI, Get Message with Automatic Ack leads to above crash:

```
[error] <0.1185.0> ** Reason for termination ==
[error] <0.1185.0> ** {function_clause,
[error] <0.1185.0>        [{mc_compat,set_annotation,
[error] <0.1185.0>             [delivery_count,2,
[error] <0.1185.0>              {basic_message,
[error] <0.1185.0>                  {resource,<<"/">>,exchange,<<>>},
[error] <0.1185.0>                  [<<"qq1">>],
[error] <0.1185.0>                  {content,60,
[error] <0.1185.0>                      {'P_basic',undefined,undefined,
[error] <0.1185.0>                          [{<<"x-delivery-count">>,long,2}],
[error] <0.1185.0>                          2,undefined,undefined,undefined,undefined,undefined,
[error] <0.1185.0>                          undefined,undefined,undefined,undefined,undefined},
[error] <0.1185.0>                      none,none,
[error] <0.1185.0>                      [<<"m1">>]},
[error] <0.1185.0>                  <<230,146,94,58,177,125,64,163,30,18,177,132,53,207,69,103>>,
[error] <0.1185.0>                  true}],
[error] <0.1185.0>             [{file,"mc_compat.erl"},{line,61}]},
[error] <0.1185.0>         {rabbit_fifo_client,add_delivery_count_header,2,
[error] <0.1185.0>             [{file,"rabbit_fifo_client.erl"},{line,228}]},
[error] <0.1185.0>         {rabbit_fifo_client,dequeue,4,
[error] <0.1185.0>             [{file,"rabbit_fifo_client.erl"},{line,211}]},
[error] <0.1185.0>         {rabbit_queue_type,dequeue,5,
[error] <0.1185.0>             [{file,"rabbit_queue_type.erl"},{line,755}]},
[error] <0.1185.0>         {rabbit_misc,with_exit_handler,2,
[error] <0.1185.0>             [{file,"rabbit_misc.erl"},{line,526}]},
[error] <0.1185.0>         {rabbit_channel,handle_method,3,
[error] <0.1185.0>             [{file,"rabbit_channel.erl"},{line,1257}]},
[error] <0.1185.0>         {rabbit_channel,handle_cast,2,
[error] <0.1185.0>             [{file,"rabbit_channel.erl"},{line,629}]},
[error] <0.1185.0>         {gen_server2,handle_msg,2,[{file,"gen_server2.erl"},{line,1056}]}]}
```

The mc annotation `delivery_count` is a new mc annotation specifically
used in the header section of AMQP 1.0 messages:
https://docs.oasis-open.org/amqp/core/v1.0/os/amqp-core-messaging-v1.0-os.html#type-header

Hence, we can ignore this annotation for the old `#basic_message{}`.
  • Loading branch information
ansd committed Sep 30, 2024
1 parent 4da8dd5 commit 36a84f4
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 2 deletions.
6 changes: 5 additions & 1 deletion deps/rabbit/src/mc_compat.erl
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,11 @@ set_annotation(?ANN_TIMESTAMP, Millis,
#basic_message{content = #content{properties = B} = C0} = Msg) ->
C = C0#content{properties = B#'P_basic'{timestamp = Millis div 1000},
properties_bin = none},
Msg#basic_message{content = C}.
Msg#basic_message{content = C};
set_annotation(delivery_count, _Value, #basic_message{} = Msg) ->
%% Ignore AMQP 1.0 specific delivery-count.
%% https://github.com/rabbitmq/rabbitmq-server/issues/12398
Msg.

is_persistent(#basic_message{content = Content}) ->
get_property(durable, Content).
Expand Down
9 changes: 8 additions & 1 deletion deps/rabbit/test/mc_unit_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,14 @@ amqpl_compat(_Config) ->

XName= <<"exch">>,
RoutingKey = <<"apple">>,
{ok, Msg} = rabbit_basic:message_no_id(XName, RoutingKey, Content),
{ok, Msg00} = rabbit_basic:message_no_id(XName, RoutingKey, Content),

%% Quorum queues set the AMQP 1.0 specific annotation delivery_count.
%% This should be a no-op for mc_compat.
Msg0 = mc:set_annotation(delivery_count, 1, Msg00),
%% However, annotation x-delivery-count has a meaning for mc_compat messages.
Msg = mc:set_annotation(<<"x-delivery-count">>, 2, Msg0),
?assertEqual({long, 2}, mc:x_header(<<"x-delivery-count">>, Msg)),

?assertEqual(98, mc:priority(Msg)),
?assertEqual(false, mc:is_persistent(Msg)),
Expand Down

0 comments on commit 36a84f4

Please sign in to comment.