Skip to content

XHTTP client: Define Request.GetBody() for packet-up so h2 can replay after GOAWAY - #6632

Merged
RPRX merged 2 commits into
XTLS:mainfrom
FunLay123:xhttp_uplink_fix
Aug 26, 2026
Merged

RPRX merged 2 commits into
XTLS:mainfrom
FunLay123:xhttp_uplink_fix

Conversation

@FunLay123

@FunLay123 FunLay123 commented Aug 16, 2026 •

Copy link
Copy Markdown
Contributor

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

这个重试看起来还行

@FunLay123
FunLay123 changed the base branch from main to close-trace August 23, 2026 09:26
@FunLay123
FunLay123 changed the base branch from close-trace to main August 23, 2026 09:31
@RPRX

RPRX commented Aug 26, 2026

Copy link
Copy Markdown
Member

涉及到了之前关注过的 Go HTTP client 会不会自动缓存 body 并在没收到 200 response 时自动重发的问题(理论上对于 POST 不该有这种行为),此外还涉及客户端内存占用问题

现在看起来的确没有无脑自动重新 POST,而是需要自定义 GetBody() 来决定?不过它是否只适用于 h2 GOAWAY?此外这样改的话确实解决了一个边缘的内存泄漏问题,代价是多 copy 了一次,以后可以优化掉,并且占用内存的时间看起来会更长一些,但要考虑重发的话也只能这样了;仍然不确定的是 Go HTTP client 内部的 body 何时释放,期待你的讨论

@RPRX RPRX changed the title XHTTP: define Request.GetBody for packet-up so h2 can replay after GOAWAY XHTTP client: Define Request.GetBody() for packet-up so h2 can replay after GOAWAY Aug 26, 2026
@RPRX
RPRX merged commit dffc7ad into XTLS:main Aug 26, 2026
40 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.

3 participants