Skip to content

Commit 6910c6c

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 3a6a2f2 commit 6910c6c

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
@@ -945,17 +945,14 @@ handle_control(#'v1_0.flow'{handle = Handle} = Flow,
945945
end
946946
end;
947947

948-
handle_control(#'v1_0.detach'{handle = Handle = ?UINT(HandleInt),
949-
closed = Closed},
948+
handle_control(Detach = #'v1_0.detach'{handle = ?UINT(HandleInt)},
950949
State0 = #state{queue_states = QStates0,
951950
incoming_links = IncomingLinks,
952951
outgoing_links = OutgoingLinks0,
953952
outgoing_unsettled_map = Unsettled0,
954953
cfg = #cfg{
955-
writer_pid = WriterPid,
956954
vhost = Vhost,
957-
user = #user{username = Username},
958-
channel_num = Ch}}) ->
955+
user = #user{username = Username}}}) ->
959956
Ctag = handle_to_ctag(HandleInt),
960957
%% TODO delete queue if closed flag is set to true? see 2.6.6
961958
%% TODO keep the state around depending on the lifetime
@@ -1011,8 +1008,7 @@ handle_control(#'v1_0.detach'{handle = Handle = ?UINT(HandleInt),
10111008
incoming_links = maps:remove(HandleInt, IncomingLinks),
10121009
outgoing_links = OutgoingLinks,
10131010
outgoing_unsettled_map = Unsettled},
1014-
rabbit_amqp_writer:send_command(WriterPid, Ch, #'v1_0.detach'{handle = Handle,
1015-
closed = Closed}),
1011+
maybe_detach_reply(Detach, State, State0),
10161012
publisher_or_consumer_deleted(State, State0),
10171013
{noreply, State};
10181014

@@ -2192,6 +2188,22 @@ publisher_or_consumer_deleted(
21922188
ok
21932189
end.
21942190

2191+
%% If we previously already sent a detach with an error condition, and the Detach we
2192+
%% receive here is therefore the client's reply, do not reply again with a 3rd detach.
2193+
maybe_detach_reply(Detach,
2194+
#state{incoming_links = NewIncomingLinks,
2195+
outgoing_links = NewOutgoingLinks,
2196+
cfg = #cfg{writer_pid = WriterPid,
2197+
channel_num = Ch}},
2198+
#state{incoming_links = OldIncomingLinks,
2199+
outgoing_links = OldOutgoingLinks})
2200+
when map_size(NewIncomingLinks) < map_size(OldIncomingLinks) orelse
2201+
map_size(NewOutgoingLinks) < map_size(OldOutgoingLinks) ->
2202+
Reply = Detach#'v1_0.detach'{error = undefined},
2203+
rabbit_amqp_writer:send_command(WriterPid, Ch, Reply);
2204+
maybe_detach_reply(_, _, _) ->
2205+
ok.
2206+
21952207
check_internal_exchange(#exchange{internal = true,
21962208
name = XName}) ->
21972209
protocol_error(?V_1_0_AMQP_ERROR_UNAUTHORIZED_ACCESS,

0 commit comments

Comments
 (0)