Skip to content

Commit 863f033

Browse files
committed
Fix channel reuse bug
This commit fixes the following test flake that occurred in CI: ``` make -C deps/rabbit ct-amqp_dotnet t=cluster_size_1:redelivery ``` After receiving the end frame, the server session proc replies with the end frame. Usually when the test case succeeds, the server connection process receives a DOWN for the session proc and untracks its channel number such that a subsequent begin frame for the same channel number will create a new session proc in the server. In the flake however, the client receives the end, and pipelines new begin, attach, and flow frames. These frames are received in the server connection's mailbox before the monitor for the old session proc fires. That's why these new frames are sent to the old session proc causing the test case to fail. This reveals a bug in the server. This commit fixes this bug similarly as done in the AMQP 0.9.1 channel in https://github.com/rabbitmq/rabbitmq-server/blob/94b4a6aafdfac6b6cae102f50b188e5ea4a32c0e/deps/rabbit/src/rabbit_channel.erl#L1146-L1155 Channel reuse by the client is valid and actually common, e.g. if channel-max is 0. (cherry picked from commit 6413d2d)
1 parent 7c3f948 commit 863f033

File tree

2 files changed

+15
-4
lines changed

2 files changed

+15
-4
lines changed

deps/rabbit/src/rabbit_amqp_reader.erl

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@
1717
-export([init/1,
1818
info/2,
1919
mainloop/2,
20-
set_credential/2]).
20+
set_credential/2,
21+
notify_session_ending/3]).
2122

2223
-export([system_continue/3,
2324
system_terminate/4,
@@ -79,6 +80,11 @@ set_credential(Pid, Credential) ->
7980
Pid ! {set_credential, Credential},
8081
ok.
8182

83+
-spec notify_session_ending(pid(), pid(), non_neg_integer()) -> ok.
84+
notify_session_ending(ConnPid, SessionPid, ChannelNum) ->
85+
ConnPid ! {session_ending, SessionPid, ChannelNum},
86+
ok.
87+
8288
%%--------------------------------------------------------------------------
8389

8490
recvloop(Deb, State = #v1{pending_recv = true}) ->
@@ -233,6 +239,8 @@ handle_other({set_credential, Cred}, State) ->
233239
handle_other(credential_expired, State) ->
234240
Error = error_frame(?V_1_0_AMQP_ERROR_UNAUTHORIZED_ACCESS, "credential expired", []),
235241
handle_exception(State, 0, Error);
242+
handle_other({session_ending, SessionPid, ChannelNum}, State) ->
243+
untrack_channel(ChannelNum, SessionPid, State);
236244
handle_other(Other, _State) ->
237245
%% internal error -> something worth dying for
238246
exit({unexpected_message, Other}).

deps/rabbit/src/rabbit_amqp_session.erl

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -622,10 +622,11 @@ log_error_and_close_session(
622622
Error, State = #state{cfg = #cfg{reader_pid = ReaderPid,
623623
writer_pid = WriterPid,
624624
channel_num = Ch}}) ->
625-
End = #'v1_0.end'{error = Error},
626625
?LOG_WARNING("Closing session for connection ~p: ~tp",
627626
[ReaderPid, Error]),
628-
ok = rabbit_amqp_writer:send_command_sync(WriterPid, Ch, End),
627+
rabbit_amqp_reader:notify_session_ending(ReaderPid, self(), Ch),
628+
ok = rabbit_amqp_writer:send_command_sync(
629+
WriterPid, Ch, #'v1_0.end'{error = Error}),
629630
{stop, {shutdown, Error}, State}.
630631

631632
%% Batch confirms / rejects to publishers.
@@ -1162,9 +1163,11 @@ handle_frame(Detach = #'v1_0.detach'{handle = ?UINT(HandleInt)},
11621163
reply_frames(Reply, State);
11631164

11641165
handle_frame(#'v1_0.end'{},
1165-
State0 = #state{cfg = #cfg{writer_pid = WriterPid,
1166+
State0 = #state{cfg = #cfg{reader_pid = ReaderPid,
1167+
writer_pid = WriterPid,
11661168
channel_num = Ch}}) ->
11671169
State = send_delivery_state_changes(State0),
1170+
rabbit_amqp_reader:notify_session_ending(ReaderPid, self(), Ch),
11681171
ok = try rabbit_amqp_writer:send_command_sync(WriterPid, Ch, #'v1_0.end'{})
11691172
catch exit:{Reason, {gen_server, call, _ArgList}}
11701173
when Reason =:= shutdown orelse

0 commit comments

Comments
 (0)