s3: fix path double-deinit when operation throws after store creation

[robobun]

Fuzzer found a reached unreachable code panic. Fingerprints: 4ada451d887da85a, 5308c20fced315a7.

Supersedes #30495 — implements the *PathLike approach recommended in that PR's review, which fully closes the gap instead of trading the double-free for a leak on the type-getter-throws path.

Root cause

Store.initS3 / initS3WithReferencedCredentials take ownership of the PathLike via path.toThreadSafe(). For .slice_with_underlying_string, that calls BunString__toThreadSafe which creates an isolated WTFStringImpl copy and derefs the original. But the caller passed the path by value — a shallow struct copy that still points at the same impl — so the caller's PathLike is now left pointing at an impl whose ref has already been transferred away.

If a later step in the same call throws — e.g. Blob.writeFileInternal when the data argument's Symbol.toPrimitive throws, or getPresignUrlFrom with missing credentials / invalid expiresIn, or options.type getter throwing inside the constructor — the caller's errdefer path.deinit() fires and derefs the impl again, tripping bun.assert(self.hasAtLeastOneRef()) in WTFStringImpl.deref.

Minimal repro:

const throwing = { [Symbol.toPrimitive]() { throw new Error("boom"); } };
new Bun.S3Client().write("ab", throwing); // panic in debug

(Single-char paths like "x" don't crash because isolatedCopy() returns the cached single-char atom unchanged and no deref happens.)

The crash report's displayed trace points at bun_string_jsc.zig:52 / Blob.zig:4343 because Bun's panic handler prints the Zig error return trace (where error.JSError propagated) rather than the actual stack, so the real panic site in the errdefer wasn't visible.

Fix

Make the S3 construct helpers (constructS3FileWithS3CredentialsAndOptions, constructS3FileWithS3Credentials, and wrappers) take *PathLike and set it to an empty .string immediately after initS3* consumes it. This way:

• if getCredentialsWithOptions throws before initS3, the caller's path is intact and its errdefer cleans it up;
• if something throws after initS3 (either inside the constructor at getTruthyComptime("type"), or after it returns), the caller's path is already empty so its errdefer is a no-op, and the store's own cleanup frees the threadsafe copy.

This is the same ownership-transfer pattern already used by findOrCreateFileFromPath.

Covers S3Client instance methods (write/presign/exists/size/stat/unlink/file/list), the static equivalents in S3File, and Bun.file("s3://...").

[robobun]

^{Updated 11:38 AM PT - May 12th, 2026}

❌ @robobun, your commit 446fdf7 has 1 failures in Build #53783 (All Failures):

• test/js/bun/test/parallel/test-http-should-emit-close-when-connection-is-aborted.ts - timeout on 🪟 2019 x64
• test/js/bun/test/parallel/test-http-should-emit-close-when-connection-is-aborted.ts - timeout on 🪟 2019 x64-baseline
• test/js/bun/test/parallel/test-http-should-emit-close-when-connection-is-aborted.ts - timeout on 🪟 11 aarch64

---

🧪   To try this PR locally:

bunx bun-pr 30567

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

bun-30567 --bun

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 34cf38bf-4456-4c96-b871-379c7af027b9

📥 Commits

Reviewing files that changed from the base of the PR and between fb1a44c and 446fdf7.

📒 Files selected for processing (1)

• test/js/bun/s3/s3-write-throwing-data.test.ts

---

Walkthrough

Refactors S3File constructors to accept PathLike pointers and clears input paths after store init. Updates S3Client and Blob call sites to build local PathLike variables with errdefer cleanup and pass pointers. Adds tests for S3 error propagation and credential validation.

Changes

S3 Path Pointer Refactoring

Layer / File(s)	Summary
S3File constructor signatures and pointer-based initialization src/runtime/webcore/S3File.zig	Constructor helpers (constructS3FileInternalStore, constructS3FileWithS3CredentialsAndOptions, constructS3FileWithS3Credentials, constructS3FileInternal, constructInternalJS) now accept *jsc.Node.PathLike, initialize stores using path.*, and clear path.* = empty after initialization. JS-facing constructors create local PathLike with errdefer path.deinit() and pass &path into internals.
S3File path-based operations updated for pointer parameters src/runtime/webcore/S3File.zig	Operations presign, unlink, write, size, exists, and stat updated to pass pointer &path_or_blob.path.path into the refactored internal store constructor.
S3Client instance and static methods with var/errdefer cleanup src/runtime/webcore/S3Client.zig	S3Client methods (file, presign, exists, size, stat, write, unlink, listObjects, staticFile, staticListObjects) parse JS PathLike into local var with errdefer path.deinit() and pass pointers (&path, &empty_path) to S3File constructors.
Blob S3 path handling with cleanup src/runtime/webcore/Blob.zig	constructBunFile s3:// branch defers path.deinitAndUnprotect() and passes pointer &path.path into S3File.constructInternalJS.
S3 write and presign error handling tests test/js/bun/s3/s3-write-throwing-data.test.ts	New tests assert S3Client write and presign error paths propagate coercion errors from throwing objects, presign throws when expiresIn is invalid, and file construction surfaces getter-thrown errors.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main fix: preventing path double-deinit when S3 operations throw after store creation, which is the primary change across all modified files.
Description check	✅ Passed	The description comprehensively covers the root cause, minimal reproduction, and detailed fix approach, but lacks explicit 'How did you verify your code works?' section from the template.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

---

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

[github-actions]

Found 1 issue this PR may fix:

1. Alloc bit not set in pas_segregated_page_deallocate_with_page #27951 - Allocator panic ("Alloc bit not set in pas_segregated_page_deallocate_with_page") with heavy S3 usage (691 S3 operations) is consistent with the double-free of WTFStringImpl that this PR fixes

If this is helpful, copy the block below into the PR description to auto-close this issue on merge.

Fixes #27951

🤖 Generated with Claude Code

[github-actions]

This PR may be a duplicate of:

1. fix(s3): avoid double-freeing path when presign throws after store creation #30495 - Also fixes S3 path double-free when an operation throws after store creation, touching the same three files (S3File.zig, S3Client.zig, Blob.zig)

🤖 Generated with Claude Code

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/js/bun/s3/s3-write-throwing-data.test.ts`:
- Around line 15-43: Add a test that exercises the public Bun.file("s3://...")
wrapper to ensure it propagates coercion/ownership-transfer errors: mirror the
existing S3Client assertions by adding something like expect(() =>
Bun.file("s3://bucket/key", throwing)).toThrow("boom") and a variant with an
array expect(() => Bun.file("s3://bucket/key", [throwing])).toThrow("boom");
reference the Bun.file symbol and reuse the existing throwing helper and opts
from this test file so the failure is asserted on the public wrapper path.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 12cb1579-e6fa-412a-b603-940a4d7d7211

📥 Commits

Reviewing files that changed from the base of the PR and between 455c262 and f9cb610.

📒 Files selected for processing (1)

• test/js/bun/s3/s3-write-throwing-data.test.ts

[robobun]

CI status on 446fdf7 (Build #53783) and fb1a44c (Build #53775):

• ✅ test/js/bun/s3/s3-write-throwing-data.test.ts — passes on all platforms on both builds
• ✅ All other S3 tests pass
• ❌ test/js/bun/test/parallel/test-http-should-emit-close-when-connection-is-aborted.ts — timeout on windows-2019-x64 / windows-2019-x64-baseline / windows-11-aarch64 on both builds

The only hard failure on both runs is the HTTP close-on-abort timeout, a pre-existing Windows flake unrelated to this S3 change. It's also the sole/primary failure on other recent PR builds including #53771, #53760, #53751, and #53740 (all unrelated branches), and was noted on #30495 as having a fix in progress on claude/fix-http-emit-close-flake. All flaky-context retries (jsc-stress WASM mprotect, fetch-http2 AtomString, bake HMR, hot sourcemap, spawn timeout, bun-install-registry hoisting) are also unrelated to S3.

Already retriggered once (f03a4c2) with the same result — not retriggering again since this flake is hitting ~every recent Windows PR run.

[robobun]

Closing: this fix targets .zig source files, which are no longer compiled now that Bun's runtime has been ported to Rust. The corresponding Rust implementation should be checked separately if the underlying bug still reproduces.