Skip to content

XHTTP: define Request.GetBody for packet-up so h2 can replay after GOAWAY - #6632

Open
FunLay123 wants to merge 2 commits into
XTLS:mainfrom
FunLay123:xhttp_uplink_fix
Open

XHTTP: define Request.GetBody for packet-up so h2 can replay after GOAWAY#6632
FunLay123 wants to merge 2 commits into
XTLS:mainfrom
FunLay123:xhttp_uplink_fix

Conversation

@FunLay123

@FunLay123 FunLay123 commented Aug 16, 2026

Copy link
Copy Markdown

Not part of this PR. The download leg fails independently and earlier:
three CDNs on Russia's IP whitelist cut the idle response stream long before
the ~100s this has generally been assumed to be — VK Cloud at 10s, Timeweb
at 30s, Yandex Cloud at 60s. Cloudflare, which is not on it, is the
only one at ~125s. During an upload that leg is idle by construction.
Measurements, and the things that do not fix them:
comment on #4846.
The diff below is the uplink only.


Problem

In packet-up mode every chunk is sent as a separate POST carrying a sequence
number. FillPacketRequest assigns request.Body by hand, after the request
was created with a nil body, so Request.GetBody is never populated.

When the peer sends GOAWAY while a request body is being written,
golang.org/x/net/http2 cannot replay the request and gives up with:

http2: Transport: cannot retry err [http2: Transport received Server's graceful
shutdown GOAWAY] after Request.Body was written; define Request.GetBody to
avoid this error

For packet-up this is not a retriable loss of bytes. That seq never
arrives, and the server's reassembly queue waits for it forever — it matches
nextSeq exactly and never skips (upload_queue.go) — so the session is torn
down. dialer.go interrupts the upload pipe on any PostPacket error, which
kills the uplink outright.

Fix

Copy the payload into a plain slice once, release the pooled buffers
immediately, and serve both Body and GetBody from that copy. The else
branch of the same function already did exactly this copy, so it is hoisted and
shared instead of duplicated.

Duplicate seq values produced by a replay are harmless: uploadQueue
silently drops any packet whose Seq is below nextSeq.

Side effect: this also closes a buffer-pool leak. io.NopCloser suppressed
MultiBufferContainer.Close(), so when a request was aborted mid-write the
unread buffers were never returned to the pool. Release is now explicit and
unconditional.

Evidence

Measured against Timeweb CDN, which closes
each HTTP/2 connection after ~200 requests.

A minimal probe reproducing the pre-patch code path (Body assigned by hand,
GetBody left nil), 8 concurrent requests, 400 KB bodies, 5 minutes, routed
through the Xray tunnel itself:

  • 15 unrecoverable failures out of 1329 requests with the error above
  • 6 of 7 connections died at 201-204 requests, confirming the CDN's limit
  • failures arrive in bursts of 2-4, exactly at connection boundaries

A patched client carrying that same traffic in the same window, instrumented to
log every GetBody call:

  • 4 replays, 0 cannot retry, 0 failed to send upload over ~530 MB
    uploaded

Under test, GetBody() returns the full payload even after Body has already
been read to completion, and the splithttp suite passes unchanged.

Cost

One copy of the chunk per in-flight request, bounded by scMaxEachPostBytes.
The copy is released by GC once the request completes. GetBody itself is only
called on the retry path, so there is no cost in the normal case.

@Fangliding

Copy link
Copy Markdown
Member

这个重试看起来还行

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