make reader's handling of channel.close_ok more obviously correct

by re-introducing a call to control_throttle.

This was present in 3.0.x, and is needed there, but is actually not
needed here, due to other changes made in the area. But the reason is
quite subtle...

For control_throttle to do something here, the channel_cleanup would
have to be called for a channel for which we have run out of
credit. Now, credit is only consumed when sending content-bearing
methods to the channel with rabbit_channel:do_flow. There is a
subsequent invocation of control_throttle in the code which will set
the connection_state to 'blocking'. But this is immediately followed
by a call to post_process_frame. The frame we are looking at must be
the last frame of a content-bearing method, since otherwise the method
would not be complete and we wouldn't have passed it to the
channel. Hence that frame can only be a content_header or, more
likely, a content_body. For both of these, post_process_frame invokes
maybe_block, which will turn a 'blocking' into 'blocked'. And from
that point onwards we will no longer read anything from the socket or
process anything already in buf. So we certainly can't be processing a
channel.close_ok.

In other words, post_process_frame can only be invoked with a
channel.close_ok frame when the connection_state is 'running', or
blocking/blocked for a reason other than having run out of credit for
a channel, i.e. an alarm. Therefore forgetting about the channel as
part of the channel_cleanup call does not have an effect on
credit_flow:blocked(). And hence an invocation of control_throttle
will not alter the connection_state and is therefore unnecessary.

1 files changed, 2 insertions, 1 deletions

diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl
index f249bc9b..d2655a90 100644
--- a/src/rabbit_reader.erl
+++ b/src/rabbit_reader.erl
@@ -636,7 +636,8 @@ process_frame(Frame, Channel, State) ->
 
 post_process_frame({method, 'channel.close_ok', _}, ChPid, State) ->
     channel_cleanup(ChPid),
-    State;
+    %% this is not strictly necessary, but more obviously correct
+    control_throttle(State);
 post_process_frame({content_header, _, _, _, _}, _ChPid, State) ->
     maybe_block(State);
 post_process_frame({content_body, _}, _ChPid, State) ->