Skip to content

Commit

Permalink
Terminate reconnect_loop process when client terminates (#17)
Browse files Browse the repository at this point in the history
Resolves wooga#124
  • Loading branch information
bjosv committed Sep 24, 2020
1 parent de426bf commit 4d3866a
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 17 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Improvements and changes compared to `wooga/eredis`:
* Support of TLS introduced in Redis 6
* Changed API: `start_link` takes a proplist with options
* Correction regarding chunked error responses
* Correction regarding termination of the reconnect process
* Dialyzer corrections
* Elvis code formatting
* Improved test coverage
Expand Down
36 changes: 21 additions & 15 deletions src/eredis_client.erl
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,9 @@ maybe_reconnect(Reason, #state{queue = Queue} = State) ->
error_logger:error_msg("eredis: Re-establishing connection to ~p:~p due to ~p",
[State#state.host, State#state.port, Reason]),
Self = self(),
spawn_link(fun() -> reconnect_loop(Self, State) end),
spawn_link(fun() -> process_flag(trap_exit, true),
reconnect_loop(Self, State)
end),

%% tell all of our clients what has happened.
reply_all({error, Reason}, Queue),
Expand All @@ -491,20 +493,24 @@ maybe_reconnect(Reason, #state{queue = Queue} = State) ->
%% successfully issuing the auth and select calls. When we have a
%% connection, give the socket to the redis client.
reconnect_loop(Client, #state{reconnect_sleep=ReconnectSleep, transport=Transport}=State) ->
timer:sleep(ReconnectSleep),
case catch(connect(State)) of
{ok, #state{socket = Socket}} ->
Client ! {connection_ready, Socket},
Transport:controlling_process(Socket, Client),
Msgs = get_all_messages([]),
[Client ! M || M <- Msgs];
{error, _Reason} ->
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
_ ->
reconnect_loop(Client, State)
receive
{'EXIT', Client, Reason} -> exit(Reason)
after
ReconnectSleep ->
case catch(connect(State)) of
{ok, #state{socket = Socket}} ->
Client ! {connection_ready, Socket},
Transport:controlling_process(Socket, Client),
Msgs = get_all_messages([]),
[Client ! M || M <- Msgs];
{error, _Reason} ->
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
_ ->
reconnect_loop(Client, State)
end
end.

read_database(undefined) ->
Expand Down
3 changes: 2 additions & 1 deletion test/eredis_tcp_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -311,9 +311,10 @@ t_reconnect(Config) when is_list(Config) ->
{ok, C} = eredis:start_link("127.0.0.1", ?PORT, [{password, "wrong_password"},
{reconnect_sleep, 100},
{connect_timeout, 200}]),
timer:sleep(2000),
timer:sleep(1000), %% expect a couple of reconnect attempts
?assert(length(get_tcp_ports()) =< 1),
?assertMatch(ok, eredis:stop(C)),
timer:sleep(200), %% Wait for reconnect process to terminate
?assertEqual(0, length(get_tcp_ports())).

%%
Expand Down
3 changes: 2 additions & 1 deletion test/eredis_tls_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,10 @@ t_reconnect(Config) when is_list(Config) ->
{reconnect_sleep, 100},
{connect_timeout, 200}],
C = c_tls(ExtraOptions),
timer:sleep(2000),
timer:sleep(1000), %% expect a couple of reconnect attempts
?assert(length(get_tcp_ports()) =< 1),
?assertMatch(ok, eredis:stop(C)),
timer:sleep(200), %% Wait for reconnect process to terminate
?assertEqual(0, length(get_tcp_ports())).

%%
Expand Down

0 comments on commit 4d3866a

Please sign in to comment.