eredis icon indicating copy to clipboard operation
eredis copied to clipboard

get_all_messages only gets one message?

Open benbro opened this issue 8 years ago • 1 comments

If get_all_messages might receive more than one message, I think it should be changed to:

get_all_messages(Acc) ->
    receive
        M ->
            get_all_messages([M | Acc])
    after 0 ->
        lists:reverse(Acc)
    end.

benbro avatar Dec 17 '17 13:12 benbro

I think there is still a possible race condition.

  1. We call Client ! {connection_ready, Socket}.
  2. Client receive a redis command waiting in the mailbox.
  3. [Client ! M || M <- Msgs] is called with tcp_closed message.

Maybe we should send connection_ready only if Msgs is empty?

reconnect_loop(Client, #state{reconnect_sleep = ReconnectSleep} = State) ->
    case catch(connect(State)) of
        {ok, #state{socket = Socket}} ->
            gen_tcp:controlling_process(Socket, Client),
            case get_all_messages([]) of
              [] ->
                Client ! {connection_ready, Socket};
              _ -> 
                timer:sleep(ReconnectSleep),
                reconnect_loop(Client, State);
        {error, _Reason} ->
            timer:sleep(ReconnectSleep),
            reconnect_loop(Client, State);
        %% Something bad happened when connecting, like Redis might be
        %% loading the dataset and we got something other than 'OK' in
        %% auth or select
        _ ->
            timer:sleep(ReconnectSleep),
            reconnect_loop(Client, State)
end.

benbro avatar Dec 17 '17 14:12 benbro