From dc8dd8fb09dd8e1ed4c7799073056c8e2b9bc7d2 Mon Sep 17 00:00:00 2001 From: aboroska Date: Tue, 11 Aug 2026 09:27:20 +0100 Subject: [PATCH 1/2] Fix slow connect crashing the connection pool Convert a connection call timeout into a checkout error. The pool must not terminate just because a DNS/TCP/TLS attempt outlives its timeout. Fixes: https://github.com/benoitc/hackney/issues/927 --- src/hackney_pool.erl | 14 +++++++++++++- test/hackney_pool_tests.erl | 17 +++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/hackney_pool.erl b/src/hackney_pool.erl index d96e07a0..d14109e2 100644 --- a/src/hackney_pool.erl +++ b/src/hackney_pool.erl @@ -1014,7 +1014,7 @@ start_connection(Host, Port, Transport, Owner, Opts, State) -> case hackney_conn_sup:start_conn(ConnOpts) of {ok, Pid} -> %% Connect the connection - case hackney_conn:connect(Pid) of + case connect_connection(Pid, ConnectTimeout) of ok -> %% Monitor the process MonRef = erlang:monitor(process, Pid), @@ -1028,6 +1028,18 @@ start_connection(Host, Port, Transport, Owner, Opts, State) -> {error, Reason} end. +%% @private Convert a connection call timeout into a checkout error. The pool +%% must not terminate just because a DNS/TCP/TLS attempt outlives its timeout. +connect_connection(Pid, Timeout) -> + try hackney_conn:connect(Pid, Timeout) of + Result -> + Result + catch + exit:{timeout, _} -> + stop_conn(Pid), + {error, connect_timeout} + end. + %% @private Process a checkin - return connection to pool. %% Plain TCP connections are stored under their TCP key. An SSL upgraded %% connection is stored only when it was checked out through checkout_ssl diff --git a/test/hackney_pool_tests.erl b/test/hackney_pool_tests.erl index 0b0ac6cf..6e7d03a2 100644 --- a/test/hackney_pool_tests.erl +++ b/test/hackney_pool_tests.erl @@ -9,6 +9,8 @@ -module(hackney_pool_tests). +-export([connect/4]). + -include_lib("eunit/include/eunit.hrl"). -include("hackney.hrl"). @@ -56,6 +58,8 @@ hackney_pool_integration_test_() -> {"owner crash kills connection", fun test_owner_crash/0}, {"checkin resets owner to pool", fun test_checkin_resets_owner/0}, {"prewarm creates connections", fun test_prewarm/0}, + {"connect timeout does not crash the pool", + fun test_connect_timeout_does_not_crash_pool/0}, {"queue timeout", {timeout, 120, fun test_queue_timeout/0}}, {"checkout timeout", {timeout, 120, fun test_checkout_timeout/0}}, {"stop_pool releases in_use load_regulation slots", @@ -130,6 +134,10 @@ teardown_integration(_) -> error_logger:tty(true), ok. +connect(_Host, _Port, _Opts, _Timeout) -> + timer:sleep(100), + {error, simulated_timeout}. + setup_ssl() -> error_logger:tty(false), {ok, _} = application:ensure_all_started(cowboy), @@ -742,6 +750,15 @@ test_prewarm() -> ok = hackney_pool:stop_pool(test_pool_prewarm). +test_connect_timeout_does_not_crash_pool() -> + PoolName = test_pool_connect_timeout, + ok = hackney_pool:start_pool(PoolName, [{pool_size, 1}, {prewarm_count, 0}]), + Opts = [{pool, PoolName}, {connect_timeout, 10}, {checkout_timeout, 1000}], + ?assertEqual({error, connect_timeout}, + hackney_pool:checkout("slow.example", 443, ?MODULE, Opts)), + ?assert(is_process_alive(hackney_pool:find_pool(PoolName))), + ok = hackney_pool:stop_pool(PoolName). + %%==================================================================== %% Timeout Tests %%==================================================================== From c8c08955c72d027a84caf9f1178e2512eb20708f Mon Sep 17 00:00:00 2001 From: Benoit Chesneau Date: Tue, 11 Aug 2026 10:59:19 +0200 Subject: [PATCH 2/2] Survive a connection process dying mid-connect Only exit:{timeout, _} was caught, so a transport raising or the conn being killed while dialing still took the pool down. Catch any exit from the connect call and return it as a checkout error. The caller already stops the conn on every error return. --- src/hackney_pool.erl | 13 +++++++++---- test/hackney_pool_tests.erl | 19 +++++++++++++++++-- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/hackney_pool.erl b/src/hackney_pool.erl index d14109e2..5575104f 100644 --- a/src/hackney_pool.erl +++ b/src/hackney_pool.erl @@ -1028,16 +1028,21 @@ start_connection(Host, Port, Transport, Owner, Opts, State) -> {error, Reason} end. -%% @private Convert a connection call timeout into a checkout error. The pool -%% must not terminate just because a DNS/TCP/TLS attempt outlives its timeout. +%% @private Convert a failed connection call into a checkout error. The pool +%% must not terminate because a DNS/TCP/TLS attempt outlives its timeout, nor +%% because the connection process dies while dialing (a transport raising, or +%% the conn being killed). The caller stops the conn on any error return. connect_connection(Pid, Timeout) -> try hackney_conn:connect(Pid, Timeout) of Result -> Result catch exit:{timeout, _} -> - stop_conn(Pid), - {error, connect_timeout} + {error, connect_timeout}; + exit:{Reason, {gen_statem, call, _}} -> + {error, Reason}; + exit:Reason -> + {error, Reason} end. %% @private Process a checkin - return connection to pool. diff --git a/test/hackney_pool_tests.erl b/test/hackney_pool_tests.erl index 6e7d03a2..c999f370 100644 --- a/test/hackney_pool_tests.erl +++ b/test/hackney_pool_tests.erl @@ -60,6 +60,8 @@ hackney_pool_integration_test_() -> {"prewarm creates connections", fun test_prewarm/0}, {"connect timeout does not crash the pool", fun test_connect_timeout_does_not_crash_pool/0}, + {"connect crash does not crash the pool", + fun test_connect_crash_does_not_crash_pool/0}, {"queue timeout", {timeout, 120, fun test_queue_timeout/0}}, {"checkout timeout", {timeout, 120, fun test_checkout_timeout/0}}, {"stop_pool releases in_use load_regulation slots", @@ -134,9 +136,13 @@ teardown_integration(_) -> error_logger:tty(true), ok. -connect(_Host, _Port, _Opts, _Timeout) -> +%% Stub transport: "slow.example" outlives the connect timeout, "crash.example" +%% takes the connection process down while dialing. +connect("slow.example", _Port, _Opts, _Timeout) -> timer:sleep(100), - {error, simulated_timeout}. + {error, simulated_timeout}; +connect("crash.example", _Port, _Opts, _Timeout) -> + erlang:error(simulated_crash). setup_ssl() -> error_logger:tty(false), @@ -759,6 +765,15 @@ test_connect_timeout_does_not_crash_pool() -> ?assert(is_process_alive(hackney_pool:find_pool(PoolName))), ok = hackney_pool:stop_pool(PoolName). +test_connect_crash_does_not_crash_pool() -> + PoolName = test_pool_connect_crash, + ok = hackney_pool:start_pool(PoolName, [{pool_size, 1}, {prewarm_count, 0}]), + Opts = [{pool, PoolName}, {connect_timeout, 1000}, {checkout_timeout, 2000}], + ?assertMatch({error, {simulated_crash, _}}, + hackney_pool:checkout("crash.example", 443, ?MODULE, Opts)), + ?assert(is_process_alive(hackney_pool:find_pool(PoolName))), + ok = hackney_pool:stop_pool(PoolName). + %%==================================================================== %% Timeout Tests %%====================================================================