Stop dead-end connections instead of parking them in closed (#918)

[lambadalambda]

Fixes #918. Sibling of #902, on the path finish_sync_request/3 does not cover.

Problem

When the peer closes mid-body, read_full_body/2 returns with socket = undefined, so receiving({call, From}, body, ...) went straight to closed and never reached the reuse check added by #903:

{ok, Body, #conn_data{socket = undefined} = NewData} ->
    {next_state, closed, NewData, [{reply, From, {ok, Body}}]};

Nothing reaps it there. closed(enter) only arms the #836 grace timer when pool_pid =/= undefined, and a connection started through hackney_conn_sup:start_conn/1 has the supervisor as its owner (start_link/1 captures self(), which under a simple_one_for_one supervisor is the supervisor; connect_direct/4 and start_conn_with_socket_internal/5 pass no owner, unlike hackney_pool:start_connection/6). So the owner-DOWN clause is dead code for these, and the process parks forever holding every refc binary it read.

Callers cannot clean up: a synchronous request returns the body directly rather than a handle, and the truncated read still reports {ok, Body} — a success.

Real-world trigger: a reverse proxy aborting a large response mid-stream, fetched through a CONNECT proxy (tunnels are no_reuse and never pooled). Measured one leaked process per fetch, ~8.6 MB each.

Fix

finish_dead_request/3, for the completions that cannot continue the connection — truncated body, bodyless (204/304) response, and a failed body read — in both the body and stream_body clauses:

finish_dead_request(From, Reply, #conn_data{transport = Transport, socket = Socket,
                                            pool_pid = PoolPid} = Data) ->
    case PoolPid of
        undefined ->
            ok = close_socket(Transport, Socket),
            {stop_and_reply, normal, [{reply, From, Reply}], Data#conn_data{socket = undefined}};
        _ ->
            {next_state, closed, Data, [{reply, From, Reply}]}
    end.

A pooled connection still parks in closed, keeping the #836 grace window so late calls get a proper reply. An unpooled one closes its socket and stops. The 204/304 path also benefits: read_full_body/2 drops the socket handle without closing it, so parking leaked the fd as well.

I deliberately key on pool_pid alone rather than connection_reusable/1. Both branches here terminate — parking is "stop in 50 ms with a grace window", not reuse — so bringing reuse into it would only remove that grace from pooled non-reusable conns. finish_sync_request/3 uses connection_reusable/1 because there parking really does mean reuse; the two functions answer different questions.

Also included

get_location/1 and set_location/2 now use safe_call/2. hackney:request/5 calls set_location/2 on the original connection after a redirect chain returns; with a dead-end connection now stopping rather than parking, that pid can be gone and the bare gen_statem:call exited noproc straight out of hackney:request/5. Reproduced with pool=false, follow_redirect=true, and a 302 carrying a lying Content-Length:

• before this commit: {'EXIT',{noproc,{gen_statem,call,[<0.587.0>,{set_location,...}]}}}
• after: {ok,200,[{<<"Content-Length">>,<<"2">>}],<<"hi">>}

That exit is reachable on 4.7.2 today via the Connection: close variant, so this is worth having regardless.

Scope

This fixes one entry point into closed; it is not the whole class. Still leaking for an unpooled connection, and out of scope here:

• connected(info, {tcp_closed|ssl_closed|tcp_error|ssl_error, Socket}, ...) — an idle conn whose peer closes.
• a successful {pool, false} request, which parks in connected with nothing to stop it.

The structural fix for all of them is probably to pass owner => self() from connect_direct/4 and start_conn_with_socket_internal/5, as hackney_pool:start_connection/6 already does, so the owner-DOWN clauses become live for direct connections. That is a wider behaviour change (it would stop a connection whose creating process exits, which hackney:connect/4 users may rely on not happening), so I left it out — happy to follow up if you want it.

Tests

rebar3 eunit: all 1048 pass. Four new tests in hackney_conn_tests:

• truncated body/1 on an unpooled conn stops it, asserting exit reason normal
• same via stream_body/1
• truncated body on a pooled conn still parks in closed (guards the grace window)
• get_location/1 / set_location/2 are noproc-safe

The first fails on stock 4.7.2 (wait_down never fires). End-to-end, the redirect probe above leaves 2 connection processes on stock and 1 with this patch — the remaining one being the separate {pool, false} case noted above.