From b5c553dc7351a397e35795373a44005b81698676 Mon Sep 17 00:00:00 2001
From: Luciano Engel <luciano@nash.io>
Date: Fri, 10 Jul 2026 09:09:39 -0300
Subject: [PATCH] Stop non-reusable hackney_conn processes when a sync request
 completes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The synchronous body-completion path (`receiving({call,From}, body, ...)` and
`stream_body`) returned `{next_state, connected, ...}` unconditionally after the
body was read, never consulting `no_reuse` or `should_close_connection/1`. A
connection that must not be reused — flagged `no_reuse` (proxy tunnels, SSL
upgrades, pool disabled) or carrying `Connection: close` — therefore parked in
`connected` forever, pinned alive by its long-lived owner's link, leaking one
`hackney_conn` gen_statem process per request until the node OOMs.

The async streaming path already handles this in `finish_async_streaming/1`;
the sync path (used by e.g. httpoison's blocking requests) did not.

Add `finish_sync_request/3`, mirroring the async logic: for a non-reusable
connection, close the socket and stop the process; otherwise return to
`connected` for keep-alive reuse. Wired into both the `body` and `stream_body`
completion clauses.

Observed on a production node making a few plain-HTTP/1.1 JSON-RPC calls per
second: `hackney_conn` processes climbed 1714 -> 2539 (~1.5/s) with only ~34
live OS ports (thousands of socketless zombies stuck in `connected`). After the
fix the count oscillates near zero, returning to 0 between request bursts and
tracking live OS ports 1:1.

Fixes #902

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 src/hackney_conn.erl | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/src/hackney_conn.erl b/src/hackney_conn.erl
index 8084519a..2ff88504 100644
--- a/src/hackney_conn.erl
+++ b/src/hackney_conn.erl
@@ -1518,8 +1518,9 @@ receiving({call, From}, body, Data) ->
             %% Transition to closed state instead of connected
             {next_state, closed, NewData, [{reply, From, {ok, Body}}]};
         {ok, Body, NewData} ->
-            %% Socket still valid - return to connected state
-            {next_state, connected, NewData, [{reply, From, {ok, Body}}]};
+            %% Socket still valid - reuse it, or stop the process if the
+            %% connection must not be reused (see finish_sync_request/3).
+            finish_sync_request(From, {ok, Body}, NewData);
         {error, Reason} ->
             {next_state, closed, Data, [{reply, From, {error, Reason}}]}
     end;
@@ -1533,7 +1534,7 @@ receiving({call, From}, stream_body, Data) ->
             %% Socket was closed during body read - transition to closed state
             {next_state, closed, NewData, [{reply, From, done}]};
         {done, NewData} ->
-            {next_state, connected, NewData, [{reply, From, done}]};
+            finish_sync_request(From, done, NewData);
         {error, Reason} ->
             {next_state, closed, Data, [{reply, From, {error, Reason}}]}
     end;
@@ -1931,6 +1932,29 @@ should_close_connection(#conn_data{version = Version, response_headers = Headers
                                    request_close = RequestClose}) ->
     hackney_keepalive:should_close(Version, Headers, RequestClose).
 
+%% @private Finish a completed synchronous request. The blocking `body` /
+%% `stream_body` path historically returned unconditionally to `connected`,
+%% never consulting `no_reuse` or `should_close_connection/1`. A connection that
+%% must not be reused — flagged `no_reuse` (proxy tunnels, SSL upgrades, pool
+%% disabled) or carrying `Connection: close` per RFC 7230 — therefore parked in
+%% `connected` forever, pinned alive by its owner link, leaking one hackney_conn
+%% process per request. This mirrors finish_async_streaming/1 for the sync path:
+%% close the socket and stop the process for non-reusable connections, otherwise
+%% return to `connected` for keep-alive reuse.
+finish_sync_request(From, Reply, #conn_data{transport = Transport, socket = Socket,
+                                            no_reuse = NoReuse} = Data) ->
+    case NoReuse orelse should_close_connection(Data) of
+        true ->
+            case Socket of
+                undefined -> ok;
+                _ -> Transport:close(Socket)
+            end,
+            {stop_and_reply, normal, [{reply, From, Reply}],
+             Data#conn_data{socket = undefined}};
+        false ->
+            {next_state, connected, Data, [{reply, From, Reply}]}
+    end.
+
 %% @private Finish async streaming - close or return to connected based on Connection header
 finish_async_streaming(Data) ->
     #conn_data{transport = Transport, socket = Socket, pool_pid = PoolPid} = Data,