Skip to content
This repository has been archived by the owner on Dec 16, 2024. It is now read-only.

get_all_messages only gets one message? #107

Open
benbro opened this issue Dec 17, 2017 · 1 comment
Open

get_all_messages only gets one message? #107

benbro opened this issue Dec 17, 2017 · 1 comment

Comments

@benbro
Copy link

benbro commented Dec 17, 2017

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
Copy link
Author

benbro commented Dec 17, 2017

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant