Keep a shared HTTP/2 connection open on close/1; honour direct connect_timeout

NEWS.md

@@ -1,5 +1,20 @@
 # NEWS
 
+4.8.4 - UNRELEASED
+------------------
+
+### Fixed
+
+- `hackney:close/1` on a shared pooled HTTP/2 connection resets only the
+  caller's own streams. It stopped the connection, failing every other
+  caller's streams on it. The connection still closes itself once idle with
+  no stream open.
+- A connection opened without a pool honours `connect_timeout` above 8
+  seconds, and a dial that outlives it returns `{error, connect_timeout}`.
+  The wait was capped at 8 seconds and ended as an exit in the caller. A
+  dial stuck in the transport no longer holds the caller past its deadline
+  either (#945).
+
 4.8.3 - 2026-09-27
 ------------------
 

guides/http_guide.md

@@ -354,7 +354,7 @@ stream_loop(ConnPid) ->
 hackney:close(ConnPid).
 ```
 
-A connection from a pool stays yours between requests; it does not go back to the pool after each response. `hackney:close/1` returns it to the pool when it can be reused, and closes it otherwise.
+A connection from a pool stays yours between requests; it does not go back to the pool after each response. `hackney:close/1` returns it to the pool when it can be reused, and closes it otherwise. A pooled HTTP/2 connection is shared with other callers: `hackney:close/1` only resets your own streams on it.
 
 ### Hand the Connection to Another Process
 

src/hackney.erl

@@ -147,6 +147,7 @@ connect_direct(Transport, Host, Port, Options) ->
     undefined -> BaseConnectOpts;
     _ -> [{protocols, Protocols} | BaseConnectOpts]
   end,
+  ConnectTimeout = proplists:get_value(connect_timeout, Options, 8000),
   ConnOpts = #{
     %% The caller owns the connection: it is started under hackney_conn_sup,
     %% so without this the supervisor is the owner and the connection
@@ -155,7 +156,7 @@ connect_direct(Transport, Host, Port, Options) ->
     host => Host,
     port => Port,
     transport => Transport,
-    connect_timeout => proplists:get_value(connect_timeout, Options, 8000),
+    connect_timeout => ConnectTimeout,
     recv_timeout => proplists:get_value(recv_timeout, Options, 5000),
     %% Single-owner connection: seed the conn-level send_timeout so
     %% hackney:send_request/2 (which has no options channel) honors it.
@@ -167,7 +168,7 @@ connect_direct(Transport, Host, Port, Options) ->
   },
   case hackney_conn_sup:start_conn(ConnOpts) of
     {ok, ConnPid} ->
-      case hackney_conn:connect(ConnPid) of
+      case hackney_conn:connect(ConnPid, ConnectTimeout) of
         ok ->
           {ok, ConnPid};
         {error, Reason} ->
@@ -491,7 +492,9 @@ maybe_upgrade_ssl(_, _ConnPid, _FinalSslOpts) ->
 
 %% @private Stop a connection, tolerating an already-dead process.
 stop_conn(ConnPid) ->
-  try hackney_conn:stop(ConnPid) catch _:_ -> ok end.
+  %% Bounded: a conn wedged in a dial past its deadline is killed rather
+  %% than holding the caller.
+  try hackney_conn:stop(ConnPid, 100) catch _:_ -> ok end.
 
 %% @private Signal the websocket process to shut down, ignoring errors.
 shutdown_ws(WsPid) ->
@@ -500,8 +503,9 @@ shutdown_ws(WsPid) ->
 %% @doc Close a connection.
 -spec close(conn()) -> ok.
 close(ConnPid) when is_pid(ConnPid) ->
-  %% A pooled connection held by a connect/* caller goes back to its pool;
-  %% any other one is stopped.
+  %% A pooled connection held by a connect/* caller goes back to its pool,
+  %% and a shared HTTP/2 one only drops this caller's streams; any other one
+  %% is stopped.
   case hackney_conn:release_held(ConnPid) of
     ok -> ok;
     {error, _} -> hackney_conn:stop(ConnPid)

src/hackney_conn.erl

@@ -320,9 +320,17 @@ kill(Pid) ->
 connect(Pid) ->
     connect(Pid, ?CONNECT_TIMEOUT).
 
+%% A dial that outlives `Timeout', or a connection that dies while dialing,
+%% is an error for the caller, not an exit: the caller stops the conn on any
+%% error return.
 -spec connect(pid(), timeout()) -> ok | {error, term()}.
 connect(Pid, Timeout) ->
-    gen_statem:call(Pid, connect, Timeout).
+    try gen_statem:call(Pid, connect, Timeout)
+    catch
+        exit:{timeout, _} -> {error, connect_timeout};
+        exit:{Reason, {gen_statem, call, _}} -> {error, Reason};
+        exit:Reason -> {error, Reason}
+    end.
 
 %% @doc Get current state name for debugging.
 -spec get_state(pid()) -> {ok, atom()} | {error, term()}.
@@ -621,8 +629,11 @@ hold(Pid) ->
     catch exit:_ -> {error, closed}
     end.
 
-%% @doc Check a held connection back into its pool. Returns ok if it went
-%% back, an error if it is not a held, idle pooled connection.
+%% @doc Give up the caller's hold on a pooled connection. A held HTTP/1.1
+%% connection goes back to its pool. On a shared HTTP/2 connection only the
+%% caller's own streams are reset: the connection stays up for the other
+%% callers and closes itself once idle with no stream open. Returns an error
+%% for any other connection, which the caller then stops.
 -spec release_held(pid()) -> ok | {error, term()}.
 release_held(Pid) ->
     try gen_statem:call(Pid, release_held, 5000)
@@ -940,6 +951,17 @@ connected({call, From}, hold, #conn_data{pool_pid = PoolPid, protocol = http1} =
 connected({call, From}, hold, _Data) ->
     {keep_state_and_data, [{reply, From, ok}]};
 
+connected({call, {Caller, _} = From}, release_held,
+          #conn_data{h2_shared = true, h2_streams = Streams} = Data) ->
+    Mine = [StreamId || {StreamId, {Owner, _}} <- maps:to_list(Streams),
+                        h2_stream_owner_pid(Owner) =:= Caller],
+    Data1 = lists:foldl(
+              fun(StreamId, D) ->
+                      _ = cancel_h2_stream(D#conn_data.h2_conn, StreamId),
+                      drop_h2_stream(StreamId, D)
+              end, Data, Mine),
+    h2_stream_result(Data1, [{reply, From, ok}]);
+
 connected({call, From}, release_held, #conn_data{pool_pid = PoolPid, auto_release = false} = Data)
   when is_pid(PoolPid) ->
     connected({call, From}, release_to_pool, Data#conn_data{auto_release = true});

test/hackney_direct_connect_timeout_tests.erl

@@ -0,0 +1,52 @@
+%%% -*- erlang -*-
+%%%
+%%% This file is part of hackney released under the Apache 2 license.
+%%% See the NOTICE for more information.
+%%%
+%%% @doc connect_timeout on a connection opened without a pool (#945).
+%%%
+%%% connect_direct/4 waited on the dial with a hardcoded 8000 ms call, so a
+%%% larger connect_timeout was cut to 8 s, and a dial outliving the wait
+%%% exited the caller instead of returning an error.
+-module(hackney_direct_connect_timeout_tests).
+
+-include_lib("eunit/include/eunit.hrl").
+
+direct_connect_timeout_test_() ->
+    {setup,
+     fun() -> {ok, _} = application:ensure_all_started(hackney), ok end,
+     fun(_) -> hackney_fault_transport:clear() end,
+     [{"connect_timeout above 8 s is honoured", {timeout, 30, fun t_above_default/0}},
+      {"a dial past the deadline is an error", {timeout, 30, fun t_past_deadline/0}}]}.
+
+%% The dial takes 8.3 s, inside a 9 s connect_timeout: it must succeed.
+t_above_default() ->
+    {ok, L} = gen_tcp:listen(0, [binary, {active, false}, {ip, {127, 0, 0, 1}}]),
+    {ok, Port} = inet:port(L),
+    try
+        ok = hackney_fault_transport:set(connect, {sleep, 8300}),
+        Result = hackney:connect(hackney_fault_transport, "127.0.0.1", Port,
+                                 [{pool, false}, {connect_timeout, 9000}]),
+        ?assertMatch({ok, _}, Result),
+        {ok, Conn} = Result,
+        hackney:close(Conn)
+    after
+        hackney_fault_transport:clear(),
+        gen_tcp:close(L)
+    end.
+
+%% The transport never answers: the caller gets an error on its own deadline,
+%% not an exit, and is not held until the transport gives up.
+t_past_deadline() ->
+    try
+        ok = hackney_fault_transport:set(connect, {hang, 1500}),
+        {Elapsed, Result} =
+            timer:tc(fun() ->
+                hackney:connect(hackney_fault_transport, "127.0.0.1", 9,
+                                [{pool, false}, {connect_timeout, 50}])
+            end),
+        ?assertEqual({error, connect_timeout}, Result),
+        ?assert(Elapsed div 1000 < 800)
+    after
+        hackney_fault_transport:clear()
+    end.

test/hackney_http2_shared_conn_tests.erl

@@ -28,7 +28,11 @@ shared_conn_test_() ->
         {"an unregistered connection still releases its slot",
          fun unregistered_conn_releases_slot/0},
         {"an unpooled connection is not shared",
-         fun unpooled_conn_not_shared/0}
+         fun unpooled_conn_not_shared/0},
+        {"close/1 by one caller keeps the shared connection",
+         fun close_keeps_shared_conn/0},
+        {"close/1 resets only the caller's own stream",
+         fun close_resets_own_stream/0}
     ]].
 
 %%====================================================================
@@ -203,6 +207,46 @@ unpooled_conn_not_shared() ->
         end
     end).
 
+%% Another caller holding the shared connection through connect/2 closes
+%% it while a stream is in flight: the stream still completes.
+close_keeps_shared_conn() ->
+    with_server([], fun(URL, Port, Opts) ->
+        _ = sync_request(first, <<URL/binary, "/first">>, Opts),
+        {Handler, _} = started(<<"/first">>),
+        Conn = shared_conn(Opts, Port),
+        {ok, Conn} = hackney:connect(URL, Opts),
+        ok = hackney:close(Conn),
+        ?assert(is_process_alive(Conn)),
+        Handler ! respond,
+        ?assertMatch({ok, 200, _, <<"ok">>}, result(first))
+    end).
+
+%% A caller with its own stream open closes the connection: that stream is
+%% reset and its monitor dropped, the other caller's stream is untouched.
+close_resets_own_stream() ->
+    with_server([], fun(URL, Port, Opts) ->
+        _ = sync_request(first, <<URL/binary, "/first">>, Opts),
+        {FirstHandler, _} = started(<<"/first">>),
+        Conn = shared_conn(Opts, Port),
+        Parent = self(),
+        Closer = spawn(fun() ->
+            {ok, Conn} = hackney:connect(URL, Opts),
+            {ok, _} = hackney_conn:request_async(Conn, <<"GET">>, <<"/second">>,
+                                                 [], <<>>, true),
+            receive close -> Parent ! {closed, hackney:close(Conn)} end,
+            receive after infinity -> ok end
+        end),
+        _ = started(<<"/second">>),
+        ?assert(lists:member(Closer, monitored(Conn))),
+        Closer ! close,
+        receive {closed, R} -> ?assertEqual(ok, R) after 5000 -> error(no_close) end,
+        ?assertNot(lists:member(Closer, monitored(Conn))),
+        ?assert(is_process_alive(Conn)),
+        FirstHandler ! respond,
+        ?assertMatch({ok, 200, _, <<"ok">>}, result(first)),
+        exit(Closer, kill)
+    end).
+
 %%====================================================================
 %% Helpers
 %%====================================================================