Blob: wrap blobs for JS through JsClass::to_js only

RO
3mo ago
+254-222

Problem

  • Several native Blob producers hand JS a wrapper that reports almost nothing to the GC: estimateShallowMemoryUsageOf returns 48 for a new File(), a parsed formData() file entry, a WebSocket binaryType = "blob" message and a Bun.Image#blob() result, against 65856 for the same 64 KiB in new Blob(). A parsed FormData, whose memoryCost reads the same field, is undercounted with them.
  • Cause: the size was computed on only one of several wrapping routes (BlobExt::to_js and the Blob constructor). A blob wrapped through the by-value JsClass::to_js (Bun.Image) or wrapped directly by C++ (File, Blob__dupe, Blob__fromBytes*) never gets the field filled in, and the GC re-reads that same field for the blob's whole life.
  • Separately, BlobExt::to_js(&self) handed C++ a mutable pointer cast from a shared reference (C++ refcounts through it and frees it), and it was a safe fn that was only correct when self was exactly the Blob::new allocation. Nothing in the signature enforced that; the pattern was repeated at 16 call sites. Harmless in practice today.

Fix

  • JsClass::to_js(self) becomes the only Rust-side way to wrap a Blob: it heap-promotes and passes the Blob::new pointer straight to the plain wrapper, or to the S3File wrapper when the store is S3. BlobExt::to_js and the S3-specific helpers are deleted and every former caller passes the Blob by value; structured clone deserialization also drops its two scope guards, since the blob is now an ordinary local until the final to_js.
  • The size is computed inside Blob::new. Property to check: every wrapper, whether Rust or C++ creates it, adopts a pointer minted by Blob::new, so no wrapping route can skip the bookkeeping, and taking the Blob by value removes the heap-receiver precondition instead of documenting it.
  • Overlaps with #37667, #37697 and #31987 are spelled out in the original; this branch leaves their lines alone, and their Blob-side size calls become redundant once this lands.
  • Verification: new tests run a table of in-memory producers plus Bun.file(), Bun.stdin and the three S3 constructors through estimateShallowMemoryUsageOf, and check that only S3 blobs get the S3File wrapper. Without the fix the new File(), formData(), WebSocket and Bun.Image#blob() cases fail (each reports 48); with it the file and the neighbouring suites listed in the original pass on a debug build.

Background

  • A JS Blob is a Rust Blob struct behind a C++ wrapper. The wrapper adopts a raw heap pointer, bumps and drops the refcount through it, and frees it from its finalizer, so wrapping a blob gives the allocation away. S3-backed blobs get a wrapper subclass whose prototype adds presign, stat and bucket.
  • Blob::new is heap promotion: it boxes a by-value Blob, sets the refcount to 1 and returns the pointer a wrapper adopts. Before that a Blob is an ordinary value. Some producers (the JS constructors, Blob__dupe for FormData entries and MessageEvent, Blob__fromBytes* for WebSocket messages) return that pointer for C++ to wrap and never go through a Rust to_js.
  • reported_estimated_size is a cached in-memory footprint (held bytes plus store, content type and name; not size on disk). JSC reads it when the wrapper is allocated and again from visitChildren on every GC; it is cached because GC marking threads read it. estimateShallowMemoryUsageOf from bun:jsc returns the same number, which is how the tests observe it.
  • JsClass is the bun_jsc trait that gives a Rust type its from_js / to_js; Request already wraps itself this way. BlobExt is an extension trait in the runtime crate that layered more Blob methods, including the second to_js, on top of it.

no test proof · iteration 3 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/web/fetch/blob.test.ts

Original description

Symptom

Several native Blob producers hand JS a wrapper that reports no memory to the GC:

import { estimateShallowMemoryUsageOf } from "bun:jsc";
const bytes = new Uint8Array(64 * 1024);
estimateShallowMemoryUsageOf(new Blob([bytes]));                                      // 65856
estimateShallowMemoryUsageOf(await new Bun.Image(someBmp).png().blob());              // 48 (for a 14585 byte blob)
estimateShallowMemoryUsageOf(new File([bytes], "a.bin"));                             // 48
estimateShallowMemoryUsageOf((await new Response(formWithFile).formData()).get("f")); // 48
// a WebSocket binaryType = "blob" message: 48 as well

JSBlob reports Blob__estimatedSize(ptr) (the cached reported_estimated_size field) to JSC when the wrapper is created and again from visitChildren on every GC. If nothing computed the field before the blob was wrapped, those blobs, and containers such as a parsed FormData whose memoryCost reads the same number, stay invisible to the collector's memory accounting for their whole life.

Cause

Blob had two Rust-side ways of becoming a JS object, and the size bookkeeping was attached to one of them:

  • BlobExt::to_js(&self) (src/runtime/webcore/Blob.rs) computed the estimated size and picked the JSS3File wrapper for S3 stores, but built the pointer the wrapper adopts with ptr::from_ref(self).cast_mut(). That pointer is derived from a shared reborrow, yet it is what C++ later writes the refcount through (Blob__ref/Blob__deref; ref_count is a plain u32) and what the finalizer frees; under the aliasing model those are writes and a deallocation through a read-only pointer. It was also a safe fn whose correctness depended on self being exactly the Blob::new allocation, which nothing in the signature enforced (the shape #31987 found crashing for a stack Response). Harmless in practice today, but a contract the compiler could not see, repeated at 16 call sites as Blob::new(x) + unsafe { to_js(&*ptr) }, and the comment claiming the cast kept the Blob::new provenance was wrong.
  • JsClass::to_js(self) (src/jsc/webcore_types.rs) took the Blob by value and heap-promoted it, but skipped the bookkeeping and the S3 routing. Image.rs used it precisely to avoid the heap-receiver precondition of the other one, which is the Bun.Image symptom.
  • The C++-wrapped producers went through neither: the File constructor, Blob__dupe (how FormData entries and MessageEvent blobs are wrapped) and Blob__fromBytes* (WebSocket messages, webview screenshots) return a Blob::new pointer that C++ wraps directly, and the only explicit size computations were in BlobExt::to_js and the Blob constructor.

Fix

  1. JsClass::to_js becomes the only Rust-side wrapping path, the arrangement Request already uses for its JsClass impl: it heap-promotes and passes the Blob::new pointer straight to Blob__create or BUN__createJSS3FileUnsafely (whose extern moves next to its only caller). BlobExt::to_js, s3_file::to_js_unchecked and construct_s3_file_internal are deleted; every former caller (Blob.rs, Body.rs, Archive.rs, BunObject.rs, ObjectURLRegistry.rs, S3Client.rs, S3File.rs, Image.rs) passes the Blob by value, which removes the Blob::new + raw reborrow pair at each site. Structured clone deserialization no longer needs its two scope guards: the blob is a local until the final to_js, so an early ? drops it like any other value (Blob has no Drop impl; its store, content type and name release through their field drops, which is what deinit() did for a non-heap blob).
  2. The size is computed in Blob::new itself (calculate_estimated_byte_size / estimated_size move to webcore_types.rs as inherent methods). Every wrapper, Rust- or C++-created, adopts a pointer minted there, so this covers the C++-wrapped producers as well, and the explicit calls in to_js and in the Blob constructor go away. Two open PRs change the neighbouring lines and are complementary to this one rather than conflicting with it, so this branch leaves those lines alone: #37667 makes JSDOMFile.cpp / JSS3File.cpp report the size when the wrapper is allocated (today the S3 wrapper only re-reports from visitChildren, so it never feeds the allocation budget), and #37697 reorders the Blob__fromBytes* exports so the content type is set before promotion. Once this lands, the explicit calculate_estimated_byte_size() calls those PRs add become redundant and can be removed (the constructor line, which #37697 also edits, is the one place the diffs touch the same lines); the method stays pub for them and for types that embed a Blob by value (#37708).

Why this shape rather than marking the old function unsafe and adding the missing calls producer by producer: the precondition only existed because the function took a reference to an allocation it was about to give away, and the gaps only existed because the bookkeeping was attached to one of several wrapping routes. Taking the value removes the precondition instead of documenting it, and computing at heap promotion leaves no route around the bookkeeping, which is also why the producer-by-producer fixes in #37667 and #37697 reduce to their create-time reporting, field ordering and tests once this is in. #31987 (open) marks the old &self method unsafe; its Blob hunks are superseded by this change, the rest of that PR is unaffected.

Verification

New tests at the end of test/js/web/fetch/blob.test.ts: a table of in-memory Blob producers (new Blob, new File, slice(), structuredClone, resolveObjectURL, ReadableStream#blob() on a blob stream, buffered and streamed Response#blob(), Request#blob(), a natively parsed formData() entry, a WebSocket binaryType = "blob" message, the three Bun.Archive outputs, Bun.Image#blob()) asserting estimateShallowMemoryUsageOf(blob) >= blob.size on a 64 KiB incompressible payload; a table of store-backed producers (Bun.file(), Bun.stdin, the three S3 constructors) asserting the wrapper reports more than an empty in-memory blob does; and a check that exactly the S3 producers get the S3File wrapper. Without the fix the new File(), formData(), WebSocket and Bun.Image#blob() cases fail (each receives 48); the rest cover the call sites the change converts (every JS-reachable one: the bytes-to-bytes Bun.write branch is rejected earlier by argument validation, Bun.embeddedFiles is exercised by test/regression/issue/31575.test.ts, and the webview screenshot exports have no test here). Whole file: 96 pass on the fixed debug build (rebased on main, which added its own tests to the file in the meantime). Cross-check against the neighbouring PRs, run by applying only their test hunks to this branch: the four tests #37697 adds (parsed multipart files, a parsed file re-appended to another FormData, a WebSocket blob message, including the FormData and MessageEvent memory costs) pass against this branch as is, and #37667's File estimate test passes too, while its two allocation-time tests fail here exactly as they do on main, since reporting at allocation for the two wrappers C++ creates directly is that PR's own change.

Also run on the fixed debug build: test/js/web/fetch/{blob,body,body-clone,blob-file-name-ownership}.test.ts, test/js/web/html/FormData*.test.ts, test/js/web/structured-clone-blob-file.test.ts, test/js/node/buffer-resolveObjectURL.test.ts, test/js/web/websocket/websocket-client-short-read.test.ts, test/js/bun/archive.test.ts, test/js/bun/util/bun-stdin-slice.test.ts, test/js/web/workers/worker_blob.test.ts, test/regression/issue/31575.test.ts, the FormData/Request/Response/WebSocket cases of test/js/bun/util/heap-snapshot.test.ts, the local parts of test/js/bun/s3/s3.test.ts and the image .blob() test. cargo check for bun_bin on linux-x64 plus bun_runtime on x86_64-pc-windows-msvc (the jsc_abi_extern! sysv64 branch) and aarch64-apple-darwin, cargo clippy and cargo fmt --check on bun_jsc and bun_runtime, and test/internal/source-lints are clean.