fix(http): fix Strong reference leak in server response streaming

[sosukesuzuki]

Summary

Fix a memory leak in RequestContext.doRenderWithBody() where Strong.Impl memory was leaked when proxying streaming responses through Bun's HTTP server.

Problem

When a streaming response (e.g., from a proxied fetch request) was forwarded through Bun's server:

1. response_body_readable_stream_ref was initialized at line 1836 (from lock.readable) or line 1841 (via Strong.init())
2. For .Bytes streams with has_received_last_chunk=false, a new Strong reference was created at line 1902
3. The old Strong reference was never deinit'd, causing Strong.Impl memory to leak

This leak accumulated over time with every streaming response proxied through the server.

Solution

Add this.response_body_readable_stream_ref.deinit() before creating the new Strong reference. This is safe because:

• stream exists as a stack-local variable
• JSC's conservative GC tracks stack-local JSValues
• No GC can occur between consecutive synchronous Zig statements
• Therefore, stream won't be collected between deinit() and Strong.init()

Test

Added test/js/web/fetch/server-response-stream-leak.test.ts which:

• Creates a backend server that returns delayed streaming responses
• Creates a proxy server that forwards the streaming responses
• Makes 200 requests and checks that ReadableStream objects don't accumulate
• Fails on system Bun v1.3.5 (202 leaked), passes with the fix

Related

Similar to the Strong reference leak fixes in:

• refactor(Response) isolate body usage #23313 (fetch memory leak)
• fix(fetch): fix ReadableStream memory leak when using stream body #25846 (fetch cyclic reference leak)

[robobun]

^{Updated 7:52 AM PT - Jan 11th, 2026}

❌ @sosukesuzuki, your commit 2a9cf55 has 3 failures in Build #34524 (All Failures):

• test/js/node/http2/node-http2.test.js - 2 failing on 🪟 2019 x64-baseline
• test/js/bun/test/parallel/test-http-should-emit-close-when-connection-is-aborted.ts - timeout on 🪟 2019 x64
• test/integration/next-pages/test/dev-server.test.ts - 1 failing on 🍎 14 aarch64

---

🧪   To try this PR locally:

bunx bun-pr 25965

That installs a local version of the PR into your bun-25965 executable, so you can run:

bun-25965 --bun

[coderabbitai]

Walkthrough

Fixes memory leaks in RequestContext.zig by deinitializing existing Strong references to response_body_readable_stream before reassignment. Adds a test validating that proxy-forwarding streaming responses does not leak ReadableStream or Response objects.

Changes

Cohort / File(s)	Summary
RequestContext memory management src/bun.js/api/server/RequestContext.zig	Deinitializes existing response_body_readable_stream_ref in two locations before reassignment: in NewRequestContext prior to creating new ReadableStream.Strong, and in the Bytes branch of onBufferedBodyChunk when handling streaming ByteStream. Prevents Strong-reference leaks from previous stream assignments.
Stream leak validation test test/js/web/fetch/server-response-stream-leak.test.ts	New test file that validates Bun.serve proxy forwarding does not leak ReadableStream or Response objects. Spins up backend and proxy servers, repeatedly fetches from proxy (200 iterations), and asserts via heapStats that ReadableStream and Response object counts remain bounded (≤ 50 each).

Possibly related PRs

• fix(fetch): fix ReadableStream memory leak when using stream body #25846: Addresses similar ReadableStream Strong-reference leaks by ensuring FetchTasklet detaches its request_body, fixing memory issues in the same streaming response code path.

Suggested reviewers

• Jarred-Sumner
• cirospaciari

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main fix: addressing a Strong reference leak in server response streaming, which is the primary change across the modified files.
Description check	✅ Passed	The description comprehensively covers both template sections: 'What does this PR do?' is thoroughly explained with Problem, Solution, and Test subsections; 'How did you verify your code works?' is addressed through the detailed test description.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 35eb539 and 2a9cf55.

📒 Files selected for processing (2)

• src/bun.js/api/server/RequestContext.zig
• test/js/web/fetch/server-response-stream-leak.test.ts

🧰 Additional context used

📓 Path-based instructions (5)

**/*.zig

📄 CodeRabbit inference engine (CLAUDE.md)

In Zig code, be careful with allocators and use defer for cleanup

Files:

• src/bun.js/api/server/RequestContext.zig

src/**/*.zig

📄 CodeRabbit inference engine (src/CLAUDE.md)

src/**/*.zig: Use the # prefix for private fields in Zig structs, e.g., struct { #foo: u32 };
Use Decl literals in Zig, e.g., const decl: Decl = .{ .binding = 0, .value = 0 };
Place @import statements at the bottom of the file in Zig (auto formatter will handle positioning)
Never use @import() inline inside functions in Zig; always place imports at the bottom of the file or containing struct

Files:

• src/bun.js/api/server/RequestContext.zig

**/*.test.ts?(x)

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.test.ts?(x): Never use bun test directly - always use bun bd test to run tests with debug build changes
For single-file tests, prefer -e flag over tempDir
For multi-file tests, prefer tempDir and Bun.spawn over single-file tests
Use normalizeBunSnapshot to normalize snapshot output of tests
Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
Use tempDir from harness to create temporary directories - do not use tmpdirSync or fs.mkdtempSync
When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure
Do not write flaky tests - do not use setTimeout in tests; instead await the condition to be met
Verify tests fail with USE_SYSTEM_BUN=1 bun test <file> and pass with bun bd test <file> - tests are invalid if they pass with USE_SYSTEM_BUN=1
Test files must end with .test.ts or .test.tsx
Avoid shell commands like find or grep in tests - use Bun's Glob and built-in tools instead

Files:

• test/js/web/fetch/server-response-stream-leak.test.ts

test/**/*.test.ts?(x)

📄 CodeRabbit inference engine (CLAUDE.md)

Always use port: 0 in tests - do not hardcode ports or use custom random port number functions

Files:

• test/js/web/fetch/server-response-stream-leak.test.ts

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Use bun bd test <...test file> to run tests with compiled code changes. Do not use bun test as it will not include your changes.
Use bun:test for files ending in *.test.{ts,js,jsx,tsx,mjs,cjs}. For test files without .test extension in test/js/node/test/{parallel,sequential}/*.js, use bun bd <file> instead of bun bd test <file> since they expect exit code 0.
Do not set a timeout on tests. Bun already has timeouts built-in.

Files:

• test/js/web/fetch/server-response-stream-leak.test.ts

🧠 Learnings (19)

📓 Common learnings

Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22946
File: test/js/sql/sql.test.ts:195-202
Timestamp: 2025-09-25T22:07:13.851Z
Learning: PR oven-sh/bun#22946: JSON/JSONB result parsing updates (e.g., returning parsed arrays instead of legacy strings) are out of scope for this PR; tests keep current expectations with a TODO. Handle parsing fixes in a separate PR.

📚 Learning: 2025-10-18T20:59:45.579Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: src/bun.js/telemetry.zig:458-475
Timestamp: 2025-10-18T20:59:45.579Z
Learning: In src/bun.js/telemetry.zig, the RequestId (u64) to JavaScript number (f64) conversion in jsRequestId() is intentionally allowed to lose precision beyond 2^53-1. This is acceptable because: (1) at 1M requests/sec it takes ~285 years to overflow, (2) the counter resets per-process, and (3) these are observability IDs, not critical distributed IDs. Precision loss is an acceptable trade-off for this use case.

Applied to files:

• src/bun.js/api/server/RequestContext.zig

📚 Learning: 2026-01-05T16:32:07.551Z

Learnt from: alii
Repo: oven-sh/bun PR: 25474
File: src/bun.js/event_loop/Sigusr1Handler.zig:0-0
Timestamp: 2026-01-05T16:32:07.551Z
Learning: In Zig codebases (e.g., Bun), treat std.posix.sigaction as returning void and do not perform runtime error handling for its failure. The Zig standard library views sigaction failures as programmer errors (unreachable) because they only occur with invalid signals like SIGKILL/SIGSTOP. Apply this pattern across Zig files that call sigaction (e.g., crash_handler.zig, main.zig, filter_run.zig, process.zig) and ensure failures are not handled as recoverable errors; prefer reaching an explicit unreachable/compile-time assumption when such failures are detected.

Applied to files:

• src/bun.js/api/server/RequestContext.zig

📚 Learning: 2025-10-19T04:55:33.099Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/http/node-telemetry.test.ts:27-203
Timestamp: 2025-10-19T04:55:33.099Z
Learning: In test/js/bun/http/node-telemetry.test.ts and the Bun.telemetry._node_binding API, after the architecture refactor, the _node_binding interface only contains two methods: handleIncomingRequest(req, res) and handleWriteHead(res, statusCode). The handleRequestFinish hook and other lifecycle hooks were removed during simplification. Both current methods are fully tested.

Applied to files:

• test/js/web/fetch/server-response-stream-leak.test.ts

📚 Learning: 2025-11-24T18:36:59.706Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output

Applied to files:

• test/js/web/fetch/server-response-stream-leak.test.ts

📚 Learning: 2025-10-18T05:23:24.403Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/telemetry-server.test.ts:91-100
Timestamp: 2025-10-18T05:23:24.403Z
Learning: In the Bun codebase, telemetry tests (test/js/bun/telemetry-*.test.ts) should focus on telemetry API behavior: configure/disable/isEnabled, callback signatures and invocation, request ID correlation, and error handling. HTTP protocol behaviors like status code normalization (e.g., 200 with empty body → 204) should be tested in HTTP server tests (test/js/bun/http/), not in telemetry tests. Keep separation of concerns: telemetry tests verify the telemetry API contract; HTTP tests verify HTTP semantics.

Applied to files:

• test/js/web/fetch/server-response-stream-leak.test.ts

📚 Learning: 2026-01-05T23:04:01.518Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun bd test <...test file>` to run tests with compiled code changes. Do not use `bun test` as it will not include your changes.

Applied to files:

• test/js/web/fetch/server-response-stream-leak.test.ts

📚 Learning: 2025-10-20T01:38:02.660Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/BunFetchInstrumentation.ts:126-131
Timestamp: 2025-10-20T01:38:02.660Z
Learning: In BunFetchInstrumentation.ts, the force-restore to ORIGINAL_FETCH in the disable() method is intentionally kept (despite appearing unsafe) because it's required for proper test cleanup when instrumentation is repeatedly enabled/disabled. Without it, 15 distributed tracing and context propagation tests fail. Shimmer's unwrap() doesn't reliably restore the original fetch in Bun's globalThis context. The isBunOtelPatched safety check ensures the restore only happens when the current fetch is still ours, preventing clobbering of other tools' wrappers.

Applied to files:

• test/js/web/fetch/server-response-stream-leak.test.ts

📚 Learning: 2025-10-26T01:32:04.844Z

Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24082
File: test/cli/test/coverage.test.ts:60-112
Timestamp: 2025-10-26T01:32:04.844Z
Learning: In the Bun repository test files (test/cli/test/*.test.ts), when spawning Bun CLI commands with Bun.spawnSync for testing, prefer using stdio: ["inherit", "inherit", "inherit"] to inherit stdio streams rather than piping them.

Applied to files:

• test/js/web/fetch/server-response-stream-leak.test.ts

📚 Learning: 2026-01-05T23:04:01.518Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*-fixture.ts : Test files that spawn Bun processes should end in `*-fixture.ts` to identify them as test fixtures rather than tests themselves.

Applied to files:

• test/js/web/fetch/server-response-stream-leak.test.ts

📚 Learning: 2025-10-19T02:44:46.354Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.

Applied to files:

• test/js/web/fetch/server-response-stream-leak.test.ts

📚 Learning: 2026-01-05T23:04:01.518Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Use `bun:test` for files ending in `*.test.{ts,js,jsx,tsx,mjs,cjs}`. For test files without .test extension in test/js/node/test/{parallel,sequential}/*.js, use `bun bd <file>` instead of `bun bd test <file>` since they expect exit code 0.

Applied to files:

• test/js/web/fetch/server-response-stream-leak.test.ts

📚 Learning: 2025-09-03T01:30:58.001Z

Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: test/js/valkey/valkey.test.ts:264-271
Timestamp: 2025-09-03T01:30:58.001Z
Learning: For test/js/valkey/valkey.test.ts PUB/SUB tests, avoid arbitrary sleeps and async-forEach. Instead, resolve a Promise from the subscriber callback when the expected number of messages is observed and await it with a bounded timeout (e.g., withTimeout + Promise.withResolvers) to account for Redis server→subscriber propagation.

Applied to files:

• test/js/web/fetch/server-response-stream-leak.test.ts

📚 Learning: 2025-09-02T05:33:37.517Z

Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 22323
File: test/js/web/websocket/websocket-subprotocol.test.ts:74-75
Timestamp: 2025-09-02T05:33:37.517Z
Learning: In Bun's runtime, `await using` with Node.js APIs like `net.createServer()` is properly supported and should not be replaced with explicit cleanup. Bun has extended Node.js APIs with proper async dispose support.

Applied to files:

• test/js/web/fetch/server-response-stream-leak.test.ts

📚 Learning: 2025-11-06T00:58:23.965Z

Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24417
File: test/js/bun/spawn/spawn.test.ts:903-918
Timestamp: 2025-11-06T00:58:23.965Z
Learning: In Bun test files, `await using` with spawn() is appropriate for long-running processes that need guaranteed cleanup on scope exit or when explicitly testing disposal behavior. For short-lived processes that exit naturally (e.g., console.log scripts), the pattern `const proc = spawn(...); await proc.exited;` is standard and more common, as evidenced by 24 instances vs 4 `await using` instances in test/js/bun/spawn/spawn.test.ts.

Applied to files:

• test/js/web/fetch/server-response-stream-leak.test.ts

📚 Learning: 2025-10-30T03:48:10.513Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 24063
File: packages/bun-otel/test/context-propagation.test.ts:1-7
Timestamp: 2025-10-30T03:48:10.513Z
Learning: In Bun test files, `using` declarations at the describe block level execute during module load/parsing, not during test execution. This means they acquire and dispose resources before any tests run. For test-scoped resource management, use beforeAll/afterAll hooks instead. The pattern `beforeAll(beforeUsingEchoServer); afterAll(afterUsingEchoServer);` is correct for managing ref-counted test resources like the EchoServer in packages/bun-otel/test/ - the using block pattern should not be used at describe-block level for test resources.
<!-- [/add_learning]

Applied to files:

• test/js/web/fetch/server-response-stream-leak.test.ts

📚 Learning: 2025-10-19T04:55:27.213Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/http/node-telemetry.test.ts:27-88
Timestamp: 2025-10-19T04:55:27.213Z
Learning: In the Bun.telemetry._node_binding API for Node.js http.createServer compatibility, there are exactly two hooks: handleIncomingRequest(req, res) called once per request (returns a request ID), and handleWriteHead(res, statusCode) called once when headers are sent (explicit or implicit). The old handleRequestFinish hook no longer exists after architectural refactoring. Both hooks are tested in test/js/bun/http/node-telemetry.test.ts.

Applied to files:

• test/js/web/fetch/server-response-stream-leak.test.ts

📚 Learning: 2025-11-08T04:06:33.198Z

Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24491
File: test/js/bun/transpiler/declare-global.test.ts:17-17
Timestamp: 2025-11-08T04:06:33.198Z
Learning: In Bun test files, `await using` with Bun.spawn() is the preferred pattern for spawned processes regardless of whether they are short-lived or long-running. Do not suggest replacing `await using proc = Bun.spawn(...)` with `const proc = Bun.spawn(...); await proc.exited;`.

Applied to files:

• test/js/web/fetch/server-response-stream-leak.test.ts

📚 Learning: 2026-01-05T23:04:01.518Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2026-01-05T23:04:01.518Z
Learning: Use `await using` or `using` statements to ensure proper resource cleanup for Bun APIs like `Bun.listen`, `Bun.connect`, `Bun.spawn`, and `Bun.serve`.

Applied to files:

• test/js/web/fetch/server-response-stream-leak.test.ts

🔇 Additional comments (2)

test/js/web/fetch/server-response-stream-leak.test.ts (1)

1-52: LGTM! Well-structured leak detection test.

The test correctly validates the memory leak fix by:

• Setting up a realistic proxy-forwarding-streaming-response scenario
• Using sufficient iterations (200) to detect accumulation
• Forcing GC with the standard double-GC pattern
• Asserting bounded heap object counts

The test follows coding guidelines: await using for servers, port: 0, and proper test structure.

Optional: Consider adding warmup to reduce noise

To exclude one-time allocations from the assertion, you could add a warmup iteration:

+  // Warmup to exclude one-time allocations
+  await leak();
+  Bun.gc(true);
+
   for (let i = 0; i < 200; i++) {
     await leak();
   }

This is optional since the current threshold (≤50) already provides sufficient headroom.

src/bun.js/api/server/RequestContext.zig (1)

1898-1902: LGTM! Correct fix for the Strong reference leak.

The deinit call before reassigning response_body_readable_stream_ref prevents leaking Strong.Impl memory when forwarding streaming responses through the proxy. The fix is safe because:

1. The stream variable is stack-local and keeps the underlying object alive across the deinit
2. JSC's conservative GC tracks stack-local JSValues, preventing premature collection
3. No GC can occur between consecutive synchronous Zig statements

The comment clearly explains the rationale, and this pattern aligns with other Strong reference management throughout the file.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}