Fix connection handler issue

[mwfj]

Fix: outbound peer-close EOF delivery delayed up to 5s when response data + FIN coalesce

Fixes #48

Summary

ConnectionHandler::OnMessage() applied an inbound-only HTTP/1 half-close heuristic to all connections, including established outbound (client-role / upstream-pool) connections. When an outbound peer writes its response body and then closes the TCP connection such that the body bytes and the EOF (read() == 0) drain in the same OnMessage read cycle, the EOF was not delivered to the connection's borrower — instead a 5-second fallback deadline was armed. The borrower (the upstream proxy codec) therefore finalized the response up to 5 s late.

This PR gates the half-close heuristic to inbound connections only (connect_state_ == ConnectState::NONE); an outbound peer read-close now falls through to ForceClose(), delivering the EOF to the borrower immediately.

Root cause

In OnMessage()'s peer_closed block, the else if (callback_ran) branch arms a bounded fallback deadline. That is correct for inbound connections — HTTP/1 clients may shutdown(SHUT_WR) after sending a request while awaiting the response, so the server must not close. But the branch did not distinguish inbound from outbound, so it also fired for outbound connections, where a peer read-close after the response body is a definitive end-of-stream.

Whether the bug manifests depends on read-cycle coalescing:

• Split cycles (body bytes, then EOF in a later cycle): the EOF cycle has callback_ran == false → reaches the existing else { ForceClose(); } → EOF delivered immediately. (Already correct.)
• Coalesced cycle (body bytes appended, then read() == 0 in the same drain loop): callback_ran == true → the 5 s deadline was armed → EOF delivery delayed. (Bug.)

Coalescing is timing/platform-dependent: frequent on Linux/epoll under load (deterministic in CI), intermittent on macOS/kqueue. The fix makes the coalesced read path behave identically to the split read path — removing an OS/timing artifact from changing semantics.

The fix relies only on the existing invariant already documented in the file: inbound connections always have connect_state_ == NONE; only the outbound path (RegisterOutboundCallbacks → FinishConnect) sets CONNECTING → CONNECTED.

Impact

Any outbound/upstream response whose completion is EOF-delimited — no Content-Length and no Transfer-Encoding: chunked (HTTP/1.0 responses, Connection: close responses, streaming/SSE upstreams that end by closing the socket) — could have its finalization delayed by up to 5 s whenever the body and FIN coalesce. For a streaming upstream that drops mid-stream, the proxy's terminal/disconnect signal to the downstream client was delayed the same way. Length-delimited and chunked responses are unaffected (they complete via on_message_complete, not the peer_closed path).

Change

server/connection_handler.cc, ConnectionHandler::OnMessage():

-        } else if (callback_ran) {
+        } else if (callback_ran && connect_state_ == ConnectState::NONE) {
             // INBOUND only … arm the 5s half-close fallback deadline
             if (!has_deadline_) {
                 SetDeadline(std::chrono::steady_clock::now() +
                             std::chrono::seconds(5));
             }
         } else {
-            // No callback ran … nothing to wait for.
+            // No callback ran, OR an OUTBOUND connection whose peer read-closed
+            // after delivering its response (data + EOF coalesced into one read).
+            // A peer close on an outbound connection is a definitive end-of-stream
+            // — close now so the close path delivers the EOF to the borrower
+            // immediately, rather than waiting out the inbound-only fallback.
             ForceClose();
         }

The output_bf_.Size() > 0 flush-then-close branch is untouched, and inbound behavior is unchanged (inbound is always NONE, so callback_ran && connect_state_ == NONE ≡ callback_ran).

EOF-delivery path (unchanged terminal, just prompt)

ForceClose() → CallCloseCb() → the pool's close callback (PoolPartition::WirePoolCallbacks) relays an empty-bytes on_message_callback to the borrower → ProxyTransaction::OnUpstreamData("") → UpstreamHttpCodec::Finish() → llhttp_finish() finalizes the EOF-delimited response. This is the same terminal the 5 s-delayed path eventually reached — now delivered immediately. For HTTP/2 upstreams (also CONNECTED), a transport FIN with buffered frames likewise routes to a prompt ForceClose → MarkDead + FailAllStreams instead of a 5 s stall.

Tests

Coalescing is a genuine race that an end-to-end proxy harness cannot force reliably (the proxy often hits EAGAIN before the FIN arrives), so the primary regression guard is a deterministic unit test that buffers both the response bytes and the FIN before OnMessage runs:

• UpstreamPoolTests::TestOutboundCoalescedPeerCloseClosesPromptly (test/upstream/upstream_pool_test.h) — a socketpair written-then-shutdown(SHUT_WR) guarantees both bytes and FIN are buffered → one coalesced read cycle. Drives the connection to CONNECTED and asserts the close path fires promptly. Verified to FAIL without the fix (exact symptom: "inbound 5s half-close deadline armed for an outbound conn") and PASS with it.
• UpstreamPoolTests::TestInboundCoalescedHalfCloseDoesNotClosePromptly — companion guard locking in the preserved inbound contract (the same coalesced cycle on connect_state_ == NONE must NOT force-close).
• ProxyTests::TestIntegrationEofDelimitedResponseRelayed (test/upstream/proxy_test.h) — robust end-to-end coverage for EOF-delimited (Connection: close) proxying, a response shape that previously had no end-to-end test.

Verification

• Full suite: 1897/1897 pass (make clean && make && ./test_runner).
• upstream and proxy suites run 3× each — all pass, deterministic (no flakiness).
• race suite: 7/7 pass.
• Outbound regression test fails without the fix (bug symptom), passes with it.
• No inbound, HTTP/1 proxy, HTTP/2 upstream, keep-alive, or backpressure regressions.

Files changed

• server/connection_handler.cc — one-line gate + comment updates
• test/upstream/upstream_pool_test.h — 2 deterministic unit tests
• test/upstream/proxy_test.h — 1 end-to-end EOF-delimited test

[gemini-code-assist]

Code Review

This pull request modifies ConnectionHandler::OnMessage to ensure that outbound connections promptly close (ForceClose()) upon receiving a peer read-close (EOF), rather than incorrectly arming the inbound-only 5-second fallback deadline. This fixes a bug where coalesced response bytes and EOF on outbound connections would stall. The PR also introduces integration and unit tests to validate this behavior for both inbound and outbound scenarios. The review feedback highlights potential double-close bugs in the newly added test code, specifically in the MakeNonBlockingSocketpair helper and the test cleanup blocks, where socket file descriptors are closed but not reset to -1 before potential exception handling.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.