From 15e2be94da1b87335aa78f9876f425e95e2bdbd0 Mon Sep 17 00:00:00 2001 From: tim Date: Thu, 20 Aug 2026 02:50:16 -0700 Subject: [PATCH 1/3] fix(relay): bound the wait for upstream response headers (fixes unbounded heap growth) The relay transport sets a dial timeout, a TLS handshake timeout and an expect-continue timeout, but nothing bounds how long it waits for the upstream *response headers* after the request has been written. An upstream that accepts the connection and then never answers -- without sending FIN/RST, which is what happens when a NAT/firewall silently drops the flow or the provider hangs -- parks the goroutine in net/http.(*persistConn).roundTrip forever. That goroutine keeps the whole request alive, which in practice means three copies of the request body stay reachable for the lifetime of the process: the raw bytes from io.ReadAll in CreateBodyStorageFromReader, the decoded messages held as json.RawMessage, and the re-marshalled upstream body from common.Marshal. BodyStorageCleanup cannot help here: it runs after c.Next() returns, and for these requests c.Next() never returns. Measured on v1.0.0-rc.23 in production (see #6947 for the full evidence): - 23 goroutines stuck in persistConn.roundTrip on a single 40h-old instance, blocked between 353 and 1894 minutes (5.9h to 31.5h) - 96.9% of the live heap, sampled after a forced GC, attributable to those three body copies (HeapAlloc 892 MiB surviving three GC cycles; HeapObjects dropping 30x while bytes dropped only 25%) - the live floor grows with uptime: 33.7 MiB at 0.1h, 89.2 at 13.8h, 510.0 at 40.1h, 955.2 at 146.8h, OOMKilled at 172.9h -- same image, same config, same load Doubling the memory limit and adding GOMEMLIMIT only moved the OOM from 132h to 172.9h. RELAY_TIMEOUT (http.Client.Timeout) cannot be used for this: it covers the whole response read and would cut legitimate long streaming calls, which is why it defaults to 0. ResponseHeaderTimeout only bounds the wait for the headers; streaming after they arrive is unaffected. The default is deliberately generous. Non-streaming upstreams usually send the response headers only once generation has finished, so the value has to leave room for a long completion. 1800s is 12x shorter than the shortest hang observed here while leaving several times the headroom a normal non-streaming request needs; 0 restores the previous unbounded behaviour. The assignment goes next to the other transport.* lines rather than inside the else branch: newRelayHTTPTransport() normally takes the http.DefaultTransport.Clone() path, and DefaultTransport does not set ResponseHeaderTimeout either. This repo already sets ResponseHeaderTimeout on its other outbound transports (controller/model_sync.go, controller/ratio_sync.go); the relay path appears to have been missed. Refs #6947. Likely also the root cause of #6731, which reported the same symptom (production OOM on /v1/responses after ~64h) but was closed for template reasons. --- .env.example | 4 +++ README.md | 1 + common/constants.go | 10 ++++++ common/init.go | 1 + service/http_client.go | 12 +++++++ ...ttp_client_response_header_timeout_test.go | 32 +++++++++++++++++++ 6 files changed, 60 insertions(+) create mode 100644 service/http_client_response_header_timeout_test.go diff --git a/.env.example b/.env.example index 3b8a2a5b9534..1e0736921bb4 100644 --- a/.env.example +++ b/.env.example @@ -60,6 +60,10 @@ # RELAY_TIMEOUT=0 # Relay HTTP 客户端空闲连接超时时间，单位秒，默认跟随 Go 标准库，设置为0表示不限制 # RELAY_IDLE_CONN_TIMEOUT=90 +# 等待上游返回响应头的超时时间，单位秒，默认 1800，设置为 0 表示不限制。 +# 仅约束「等待响应头」这一段；响应头返回之后的流式传输不受影响。 +# 注意：非流式请求通常要等上游生成完毕才会返回响应头，因此该值需留足余量。 +# RELAY_RESPONSE_HEADER_TIMEOUT=1800 # 流模式无响应超时时间，单位秒，如果出现空补全可以尝试改为更大值 # STREAMING_TIMEOUT=300 diff --git a/README.md b/README.md index 91778e659d3f..d05f8e58ca71 100644 --- a/README.md +++ b/README.md @@ -327,6 +327,7 @@ docker run --name new-api -d --restart always \ | `SQL_DSN` | Database connection string | - | | `REDIS_CONN_STRING` | Redis connection string | - | | `RELAY_IDLE_CONN_TIMEOUT` | Idle keep-alive timeout for relay HTTP clients, seconds. Defaults to Go standard library behavior; set `0` to disable | `90` | +| `RELAY_RESPONSE_HEADER_TIMEOUT` | How long the relay waits for upstream **response headers**, seconds; set `0` to disable. Only bounds the header wait -- streaming after the headers arrive is unaffected. Note that non-streaming upstreams usually send headers only once generation finishes, so leave headroom | `1800` | | `STREAMING_TIMEOUT` | Streaming timeout (seconds) | `300` | | `STREAM_SCANNER_MAX_BUFFER_MB` | Max per-line buffer (MB) for the stream scanner; increase when upstream sends huge image/base64 payloads | `64` | | `MAX_REQUEST_BODY_MB` | Max request body size (MB, counted **after decompression**; prevents huge requests/zip bombs from exhausting memory). Exceeding it returns `413` | `32` | diff --git a/common/constants.go b/common/constants.go index d6b4fb52284c..4e99f5b2f7aa 100644 --- a/common/constants.go +++ b/common/constants.go @@ -162,6 +162,16 @@ var BatchUpdateInterval int var RelayTimeout int // unit is second var RelayIdleConnTimeout int // unit is second + +// RelayResponseHeaderTimeout limits how long the relay transport waits for the +// upstream response headers after the request has been fully written. +// 0 disables it (previous behaviour: wait forever). +// +// Note this is NOT the same as RelayTimeout (http.Client.Timeout), which covers +// the whole response read and therefore breaks legitimate long streaming calls. +// ResponseHeaderTimeout only bounds the wait for the response headers; once the +// headers arrive, streaming is unaffected. +var RelayResponseHeaderTimeout int // unit is second var RelayMaxIdleConns int var RelayMaxIdleConnsPerHost int diff --git a/common/init.go b/common/init.go index 4d4c62b27cac..80c6874003c2 100644 --- a/common/init.go +++ b/common/init.go @@ -110,6 +110,7 @@ func InitEnv() { BatchUpdateInterval = GetEnvOrDefault("BATCH_UPDATE_INTERVAL", 5) RelayTimeout = GetEnvOrDefault("RELAY_TIMEOUT", 0) RelayIdleConnTimeout = GetEnvOrDefault("RELAY_IDLE_CONN_TIMEOUT", 90) + RelayResponseHeaderTimeout = GetEnvOrDefault("RELAY_RESPONSE_HEADER_TIMEOUT", 1800) RelayMaxIdleConns = GetEnvOrDefault("RELAY_MAX_IDLE_CONNS", 500) RelayMaxIdleConnsPerHost = GetEnvOrDefault("RELAY_MAX_IDLE_CONNS_PER_HOST", 100) diff --git a/service/http_client.go b/service/http_client.go index f5c19eafedc8..ebcd5adfeae1 100644 --- a/service/http_client.go +++ b/service/http_client.go @@ -91,6 +91,18 @@ func newRelayHTTPTransport() *http.Transport { transport.MaxIdleConns = common.RelayMaxIdleConns transport.MaxIdleConnsPerHost = common.RelayMaxIdleConnsPerHost transport.IdleConnTimeout = time.Duration(common.RelayIdleConnTimeout) * time.Second + // Bound the wait for upstream response headers. Without it, an upstream that + // accepts the connection but never responds (and never sends FIN/RST) parks the + // goroutine forever, and every buffer that request owns -- the raw body read by + // io.ReadAll, the decoded messages, and the re-marshalled upstream body -- stays + // reachable for the lifetime of the process. + // + // This only covers the wait for the headers; streaming after the headers arrive + // is not affected. Set RELAY_RESPONSE_HEADER_TIMEOUT=0 to restore the old + // unbounded behaviour. + if common.RelayResponseHeaderTimeout > 0 { + transport.ResponseHeaderTimeout = time.Duration(common.RelayResponseHeaderTimeout) * time.Second + } transport.ForceAttemptHTTP2 = true if common.TLSInsecureSkipVerify { transport.TLSClientConfig = common.InsecureTLSConfig diff --git a/service/http_client_response_header_timeout_test.go b/service/http_client_response_header_timeout_test.go new file mode 100644 index 000000000000..f44430a15f63 --- /dev/null +++ b/service/http_client_response_header_timeout_test.go @@ -0,0 +1,32 @@ +package service + +import ( + "testing" + "time" + + "github.com/QuantumNous/new-api/common" +) + +// The relay transport must bound how long it waits for upstream response headers. +// Without it an upstream that accepts the connection but never answers parks the +// goroutine forever and keeps the whole request body reachable, which shows up as +// unbounded heap growth and eventually OOM. +func TestNewRelayHTTPTransportResponseHeaderTimeout(t *testing.T) { + original := common.RelayResponseHeaderTimeout + defer func() { common.RelayResponseHeaderTimeout = original }() + + t.Run("applies configured timeout", func(t *testing.T) { + common.RelayResponseHeaderTimeout = 42 + got := newRelayHTTPTransport().ResponseHeaderTimeout + if want := 42 * time.Second; got != want { + t.Fatalf("ResponseHeaderTimeout = %v, want %v", got, want) + } + }) + + t.Run("zero keeps it unset", func(t *testing.T) { + common.RelayResponseHeaderTimeout = 0 + if got := newRelayHTTPTransport().ResponseHeaderTimeout; got != 0 { + t.Fatalf("ResponseHeaderTimeout = %v, want 0 (disabled)", got) + } + }) +} From 8d8dfd3e6876d5ec491e7248795d07aef39d5319 Mon Sep 17 00:00:00 2001 From: tim Date: Thu, 20 Aug 2026 03:10:23 -0700 Subject: [PATCH 2/3] review: clamp overflowing timeout values and switch the test to testify Addresses the two CodeRabbit findings on this PR. Overflow (common/init.go:113): a RELAY_RESPONSE_HEADER_TIMEOUT beyond ~9.2e9 seconds overflows time.Duration and can wrap into a *tiny positive* timeout, which would cut every relay request instead of only the stuck ones. The value is now clamped before the conversion, with regression tests for both the negative and the overflowing input. I did not add fail-on-startup validation for negative values, for two reasons: the existing `if seconds > 0` guard already treats them as "disabled", and the neighbouring env-driven timeouts in this file are less strict still -- RelayIdleConnTimeout is converted with no guard at all. Failing startup on a bad value would be a behaviour change out of step with the rest of the file; happy to add it if you'd prefer that direction repo-wide. Test style: switched to testify (require.Equal / require.Zero / require.Positive), which is what every other test under service/ uses. go build, go vet and go test ./common/... ./service/... pass. (`go build ./...` fails on the `web/dist` embed both with and without this change -- the frontend bundle is not checked in.) --- service/http_client.go | 15 +++++++++-- ...ttp_client_response_header_timeout_test.go | 26 ++++++++++++++----- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/service/http_client.go b/service/http_client.go index ebcd5adfeae1..19523fb13827 100644 --- a/service/http_client.go +++ b/service/http_client.go @@ -4,6 +4,7 @@ import ( "context" "crypto/tls" "fmt" + "math" "net" "net/http" "net/url" @@ -71,6 +72,10 @@ func ValidateSSRFProtectedFetchURL(urlStr string) error { return validateURLWithCurrentFetchSetting(urlStr, true) } +// maxTimeoutSeconds is the largest number of seconds that still converts to a +// time.Duration without overflowing (~292 years). +const maxTimeoutSeconds = int(math.MaxInt64 / int64(time.Second)) + func newRelayHTTPTransport() *http.Transport { var transport *http.Transport if defaultTransport, ok := http.DefaultTransport.(*http.Transport); ok && defaultTransport != nil { @@ -100,8 +105,14 @@ func newRelayHTTPTransport() *http.Transport { // This only covers the wait for the headers; streaming after the headers arrive // is not affected. Set RELAY_RESPONSE_HEADER_TIMEOUT=0 to restore the old // unbounded behaviour. - if common.RelayResponseHeaderTimeout > 0 { - transport.ResponseHeaderTimeout = time.Duration(common.RelayResponseHeaderTimeout) * time.Second + if seconds := common.RelayResponseHeaderTimeout; seconds > 0 { + // Clamp before converting: seconds beyond maxTimeoutSeconds overflow + // time.Duration and can wrap into a tiny positive timeout, which would cut + // every relay request instead of only the stuck ones. + if seconds > maxTimeoutSeconds { + seconds = maxTimeoutSeconds + } + transport.ResponseHeaderTimeout = time.Duration(seconds) * time.Second } transport.ForceAttemptHTTP2 = true if common.TLSInsecureSkipVerify { diff --git a/service/http_client_response_header_timeout_test.go b/service/http_client_response_header_timeout_test.go index f44430a15f63..491a3015a218 100644 --- a/service/http_client_response_header_timeout_test.go +++ b/service/http_client_response_header_timeout_test.go @@ -1,10 +1,13 @@ package service import ( + "math" "testing" "time" "github.com/QuantumNous/new-api/common" + + "github.com/stretchr/testify/require" ) // The relay transport must bound how long it waits for upstream response headers. @@ -17,16 +20,25 @@ func TestNewRelayHTTPTransportResponseHeaderTimeout(t *testing.T) { t.Run("applies configured timeout", func(t *testing.T) { common.RelayResponseHeaderTimeout = 42 - got := newRelayHTTPTransport().ResponseHeaderTimeout - if want := 42 * time.Second; got != want { - t.Fatalf("ResponseHeaderTimeout = %v, want %v", got, want) - } + require.Equal(t, 42*time.Second, newRelayHTTPTransport().ResponseHeaderTimeout) }) t.Run("zero keeps it unset", func(t *testing.T) { common.RelayResponseHeaderTimeout = 0 - if got := newRelayHTTPTransport().ResponseHeaderTimeout; got != 0 { - t.Fatalf("ResponseHeaderTimeout = %v, want 0 (disabled)", got) - } + require.Zero(t, newRelayHTTPTransport().ResponseHeaderTimeout) + }) + + t.Run("negative keeps it unset", func(t *testing.T) { + common.RelayResponseHeaderTimeout = -1 + require.Zero(t, newRelayHTTPTransport().ResponseHeaderTimeout) + }) + + // A value large enough to overflow time.Duration must not wrap into a tiny + // positive timeout, which would cut every relay request. + t.Run("overflowing value is clamped", func(t *testing.T) { + common.RelayResponseHeaderTimeout = math.MaxInt64 + got := newRelayHTTPTransport().ResponseHeaderTimeout + require.Positive(t, got) + require.Equal(t, time.Duration(maxTimeoutSeconds)*time.Second, got) }) } From 1b9124ed2cc35c94cb5ca21085f12f2a64d03ea1 Mon Sep 17 00:00:00 2001 From: Calcium-Ion Date: Sun, 30 Aug 2026 21:18:01 +0800 Subject: [PATCH 3/3] Delete service/http_client_response_header_timeout_test.go --- ...ttp_client_response_header_timeout_test.go | 44 ------------------- 1 file changed, 44 deletions(-) delete mode 100644 service/http_client_response_header_timeout_test.go diff --git a/service/http_client_response_header_timeout_test.go b/service/http_client_response_header_timeout_test.go deleted file mode 100644 index 491a3015a218..000000000000 --- a/service/http_client_response_header_timeout_test.go +++ /dev/null @@ -1,44 +0,0 @@ -package service - -import ( - "math" - "testing" - "time" - - "github.com/QuantumNous/new-api/common" - - "github.com/stretchr/testify/require" -) - -// The relay transport must bound how long it waits for upstream response headers. -// Without it an upstream that accepts the connection but never answers parks the -// goroutine forever and keeps the whole request body reachable, which shows up as -// unbounded heap growth and eventually OOM. -func TestNewRelayHTTPTransportResponseHeaderTimeout(t *testing.T) { - original := common.RelayResponseHeaderTimeout - defer func() { common.RelayResponseHeaderTimeout = original }() - - t.Run("applies configured timeout", func(t *testing.T) { - common.RelayResponseHeaderTimeout = 42 - require.Equal(t, 42*time.Second, newRelayHTTPTransport().ResponseHeaderTimeout) - }) - - t.Run("zero keeps it unset", func(t *testing.T) { - common.RelayResponseHeaderTimeout = 0 - require.Zero(t, newRelayHTTPTransport().ResponseHeaderTimeout) - }) - - t.Run("negative keeps it unset", func(t *testing.T) { - common.RelayResponseHeaderTimeout = -1 - require.Zero(t, newRelayHTTPTransport().ResponseHeaderTimeout) - }) - - // A value large enough to overflow time.Duration must not wrap into a tiny - // positive timeout, which would cut every relay request. - t.Run("overflowing value is clamped", func(t *testing.T) { - common.RelayResponseHeaderTimeout = math.MaxInt64 - got := newRelayHTTPTransport().ResponseHeaderTimeout - require.Positive(t, got) - require.Equal(t, time.Duration(maxTimeoutSeconds)*time.Second, got) - }) -}