Repository navigation
fix(forge): cancel a vanished client's transport within seconds, not after 120 s - #334
Merged
Merged
Conversation
A client that left mid-request without a close the server received (seen on macOS loopback in about 1 run in 8: the socket stayed ESTABLISHED and the body read never saw FIN or RST) held its transport until the 120 s idle timeout. Accepted sockets now enable TCP keepalive with a 15 s idle time (Limits::tcp_keepalive), so the probes find the peer gone. Setting it is best effort: a failure logs a warning and the connection is served. The mid-pack cancel test now runs with the default idle timeout and a 1 s probe delay (cleanup took at most 1.14 s over 12 runs, against 120 s in 2 of 6 before); a new test shows a paused live client survives the probes.
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars.
 |
tonibergholm
approved these changes
Nov 9, 2026
5 of 8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A client that disconnected mid-push could hold its git transport until the 120 s idle timeout. In about 1 run in 8 on macOS, the server never received the close: the socket stayed
ESTABLISHEDwith empty queues, and hyper's body read saw no FIN or RST. Normal runs get a reset and cancel within about 50 ms. #316 tracked this.The fix enables TCP keepalive on accepted sockets, with probes after 15 s of silence (
server::Limits::tcp_keepalive, the same default as Go'snet/httpserver). A probe to a vanished peer gets a reset, so the transport is cancelled. A paused but live client answers the probes. Setting keepalive is best effort: if it fails, the server logs a warning and still serves the connection.keepalive_probes_leave_a_paused_live_client_alone: a stalled live push keeps its transport for 4 s with a 1 s probe delay.docs/THREAT-MODEL.mdnow describes keepalive's limits. An unanswered probe follows the platform's retry defaults, so--git-idle-timeoutstill bounds a blackholed peer. Behind a proxy, the probes reach only the proxy.Proof: before the fix, 2 of 6 runs took 120 s from disconnect to cleanup. After it, the slowest of 12 runs took 1.14 s. Three runs on Linux with Rust 1.96.1 pass.
cargo test -p scl-forgepasses (321 tests), and so do fmt and clippy (-D warnings).Not established: why the macOS loopback close goes missing. A packet capture needed sudo. Sol's diagnosis ruled out hyper's body channel and backpressure.
Reviewed with sol-loop: 1 round with gpt-6.1-sol on high. Sol verdict: approved. gpt-6.1-sol also diagnosed the bug and prototyped the fix.
Created with Claude Opus 5.5 in T3 Code (Claude Code harness).