Skip to content

Commit 18e19ff

Browse files
committed
Fix bug where server sends a 3rd detach.
If the server initiates the detach due to an error condition, it destroys and therefore forgets the link. This should be okay because accroding to section 2.6.5: "When an error occurs at a link endpoint, the endpoint MUST be detached with appropriate error information supplied in the error field of the detach frame. The link endpoint MUST then be destroyed." It is also valid that the client replies with a detach: "If any input (other than a detach) related to the endpoint either via the input handle or delivery-ids be received, the session MUST be terminated with an errant-link session-error." In this case, the server must not reply again with (i.e do not sent a 3rd) detach.
1 parent 25eb318 commit 18e19ff

File tree

1 file changed

+19
-7
lines changed

1 file changed

+19
-7
lines changed

deps/rabbit/src/rabbit_amqp_session.erl

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1079,17 +1079,14 @@ handle_control(#'v1_0.flow'{handle = Handle} = Flow,
10791079
end,
10801080
{noreply, S};
10811081

1082-
handle_control(#'v1_0.detach'{handle = Handle = ?UINT(HandleInt),
1083-
closed = Closed},
1082+
handle_control(Detach = #'v1_0.detach'{handle = ?UINT(HandleInt)},
10841083
State0 = #state{queue_states = QStates0,
10851084
incoming_links = IncomingLinks,
10861085
outgoing_links = OutgoingLinks0,
10871086
outgoing_unsettled_map = Unsettled0,
10881087
cfg = #cfg{
1089-
writer_pid = WriterPid,
10901088
vhost = Vhost,
1091-
user = #user{username = Username},
1092-
channel_num = Ch}}) ->
1089+
user = #user{username = Username}}}) ->
10931090
Ctag = handle_to_ctag(HandleInt),
10941091
%% TODO delete queue if closed flag is set to true? see 2.6.6
10951092
%% TODO keep the state around depending on the lifetime
@@ -1145,8 +1142,7 @@ handle_control(#'v1_0.detach'{handle = Handle = ?UINT(HandleInt),
11451142
incoming_links = maps:remove(HandleInt, IncomingLinks),
11461143
outgoing_links = OutgoingLinks,
11471144
outgoing_unsettled_map = Unsettled},
1148-
rabbit_amqp_writer:send_command(WriterPid, Ch, #'v1_0.detach'{handle = Handle,
1149-
closed = Closed}),
1145+
maybe_detach_reply(Detach, State, State0),
11501146
publisher_or_consumer_deleted(State, State0),
11511147
{noreply, State};
11521148

@@ -2457,6 +2453,22 @@ publisher_or_consumer_deleted(
24572453
ok
24582454
end.
24592455

2456+
%% If we previously already sent a detach with an error condition, and the Detach we
2457+
%% receive here is therefore the client's reply, do not reply again with a 3rd detach.
2458+
maybe_detach_reply(Detach,
2459+
#state{incoming_links = NewIncomingLinks,
2460+
outgoing_links = NewOutgoingLinks,
2461+
cfg = #cfg{writer_pid = WriterPid,
2462+
channel_num = Ch}},
2463+
#state{incoming_links = OldIncomingLinks,
2464+
outgoing_links = OldOutgoingLinks})
2465+
when map_size(NewIncomingLinks) < map_size(OldIncomingLinks) orelse
2466+
map_size(NewOutgoingLinks) < map_size(OldOutgoingLinks) ->
2467+
Reply = Detach#'v1_0.detach'{error = undefined},
2468+
rabbit_amqp_writer:send_command(WriterPid, Ch, Reply);
2469+
maybe_detach_reply(_, _, _) ->
2470+
ok.
2471+
24602472
check_internal_exchange(#exchange{internal = true,
24612473
name = XName}) ->
24622474
protocol_error(?V_1_0_AMQP_ERROR_UNAUTHORIZED_ACCESS,

0 commit comments

Comments
 (0)