Skip to content

fix: bound stalled writes on shared H2 connections - #30

Merged
cppla merged 1 commit into
mainfrom
codex/h2-write-stall-timeout
Sep 23, 2026
Merged

cppla merged 1 commit into
mainfrom
codex/h2-write-stall-timeout

Conversation

@cppla

@cppla cppla commented Sep 23, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Bound writes on the shared H2 physical connection with http2.Transport.WriteByteTimeout, preventing an unread response close from waiting indefinitely behind a stalled connection writer.
  • Add WebH2ClientConfig.WriteByteTimeout / WebClientConfig.H2WriteByteTimeout and the shared CLI/JSON h2-write-timeout setting. Zero uses a 30-second default; negative durations are rejected. Native and H3 transports are unchanged.
  • Document physical-connection failure semantics, TLS partial-progress limitations, and source-build/v1.0.1 boundaries.

Regression evidence

  • A real HTTP/2 frame fixture reproduces current-session Close and deadline cleanup blocked behind a SETTINGS ACK with unread DATA. The production constructor's configured transport now releases those paths, closes the failed wire and returns stream ownership.
  • Timeout-disabled negative controls still fail the bounded current-session assertions; retired-session behavior remains working.
  • Actual verified native TLS and Chrome uTLS sessions run through a controlled local byte bridge. Pausing its client-to-server reads produces real raw net.Pipe.Write deadline errors; the affected connection fails and a subsequent Dial authenticates a fresh session without manually changing client state.
  • Healthy sessions survive idle periods longer than the test timeout, continuous download after upload half-close, sibling traffic and connection reuse.
  • CLI/JSON/default/negative/offline preflight tests, existing initialization cancellation/handoff tests, full make check, full make race, repeated targeted race tests and independent review passed.
  • Isolated Linux/arm64 Docker targeted tests passed: 8 top-level tests x3 (24 passes), including both TLS variants; all 68 tunnel Go source hashes were unchanged across build and execution, and the container was removed.
  • Exact head 6e6579ef037022ed7eff37b456eff873501d64ff passed CI (all 11 jobs), CodeQL, and Linux namespace/netem. Docker integration and OCI export steps ran successfully; their logs and the netem success marker were checked. No open review threads were present at this verification.

Semantics and scope

This is shared physical-write protection, not an idle timeout or a stream-local deadline. TLS write timeout can terminate all streams on that connection, including after partial network progress. Normal per-stream cancellation remains isolated. Stream Close can wait for the pending write budget and cleanup; this is bounded recovery, not an immediate-Close guarantee. The 30-second operational default is configurable, not a claim of a universally optimal value.

No dependency, wire authentication, congestion-controller, release or tag change. The controlled byte bridge is a deterministic correctness fixture, not an Internet throughput or browser-similarity experiment.

Merge receipt

Merged as 45bd8e54f38a846ded9144da0004a7acf1e19681. Its tree 0e30f4f389f2b6916af0166060f26f236218ba34 is identical to the tested PR head. Exact-main push CI (all 11 jobs), CodeQL, and Linux namespace/netem all passed. Main Docker integration, OCI export and netem logs were checked. Local main is synchronized; no release or tag was created.

Copilot AI lite review requested due to automatic review settings September 23, 2026 03:19

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Shared connection failure and TLS timeout semantics warrant final human review.

Review effort: Lite
Findings: None

What changed in this PR

Adds configurable HTTP/2 physical-write timeouts to bound stalled shared connections and improve cleanup.

Changes:

  • Wires timeout configuration through clients, CLI, and JSON.
  • Adds lifecycle, TLS, recovery, and healthy-session tests.
  • Documents timeout behavior and operational semantics.
File Description
internal/​tunnel/​web_h2_write_timeout_test.go Tests stalled-write cleanup.
internal/​tunnel/​web_h2_write_timeout_integration_test.go Tests TLS behavior and recovery.
internal/​tunnel/​web_h2_client.go Configures and validates H2 write timeouts.
internal/​tunnel/​web_client.go Propagates timeout to H2 fallback.
docs/​WEB_COVER.md Documents timeout behavior.
docs/​DEPLOYMENT.md Documents operational semantics.
cmd/​autocar/​h2_write_timeout_test.go Tests CLI, JSON, defaults, and validation.
cmd/​autocar/​common.go Adds CLI configuration and wiring.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@cppla
cppla merged commit 45bd8e5 into main Sep 23, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants