diff --git a/src/jsc/webcore_types.rs b/src/jsc/webcore_types.rs index b3a6a52daae4..c67ca8e0c75c 100644 --- a/src/jsc/webcore_types.rs +++ b/src/jsc/webcore_types.rs @@ -190,6 +190,16 @@ const _: () = { safe fn __bun_blob_from_build_artifact(value: JSValue) -> Option<*mut Blob>; } + // `JSS3File` (src/jsc/bindings/JSS3File.cpp) is the `JSBlob` subclass whose + // prototype carries `presign` / `stat` / `bucket`. Like `Blob__create` it + // adopts `blob` as `m_ctx`; the `JSBlob` finalizer releases it. + crate::jsc_abi_extern! { + fn BUN__createJSS3FileUnsafely( + global: &JSGlobalObject, + blob: *mut core::ffi::c_void, + ) -> JSValue; + } + impl JsClass for Blob { fn from_js(value: JSValue) -> Option<*mut Self> { JSBlob::from_js(value).or_else(|| __bun_blob_from_build_artifact(value)) @@ -197,12 +207,19 @@ const _: () = { fn from_js_direct(value: JSValue) -> Option<*mut Self> { JSBlob::from_js_direct(value) } + /// The Rust-side way to wrap a `Blob` (the JS constructors and the + /// `Blob__dupe` / `Blob__fromBytes*` exports instead return the + /// [`Blob::new`] pointer for C++ to wrap): heap-promotes `self` and gives + /// the allocation to the wrapper, which owns it from then on + /// ([`Blob::finalize`]). S3-backed blobs get the `JSS3File` subclass. fn to_js(self, global: &JSGlobalObject) -> JSValue { - // Heap-promote and hand - // ownership to the codegen wrapper. The S3File fast-path (different - // JS wrapper) is layered on by `bun_runtime`'s `BlobExt::to_js` for - // S3-backed blobs; lower-tier callers never construct S3 blobs. + let is_s3 = self.is_s3(); let ptr = Blob::new(self); + if is_s3 { + // SAFETY: `ptr` is the allocation `Blob::new` just returned and + // nothing else holds it; the wrapper becomes its sole owner. + return unsafe { BUN__createJSS3FileUnsafely(global, ptr.cast()) }; + } JSBlob::to_js(ptr, global) } fn get_constructor(global: &JSGlobalObject) -> JSValue { @@ -212,10 +229,15 @@ const _: () = { }; impl Blob { - /// Heap-promote and mark as - /// heap-allocated so `deinit` knows to free the heap box. + /// Heap-promote and mark as heap-allocated so `deinit` knows to free the + /// heap box. Every JS wrapper, whether created from Rust ([`JsClass::to_js`]) + /// or from C++ (the constructors, `Blob__dupe`, `Blob__fromBytes*`), adopts + /// a pointer minted here and reports `reported_estimated_size` to the GC + /// from then on, so this is where the size is computed: only the store, + /// content type and name set before this call are counted. #[inline] pub fn new(mut blob: Blob) -> *mut Blob { + blob.calculate_estimated_byte_size(); blob.ref_count = bun_ptr::RawRefCount::init(1); bun_core::heap::into_raw(Box::new(blob)) } @@ -451,6 +473,47 @@ impl Blob { } } + /// Compute `reported_estimated_size`: the in-memory footprint (not the size + /// on disk) that [`Self::estimated_size`] reports. [`Blob::new`] calls this + /// for every heap blob; a type that embeds a `Blob` by value and reports it + /// as part of its own size calls it directly. The GC reads the result from + /// its marking threads once the blob is reachable, which is why it is a + /// value cached up front rather than a walk of the store at report time. + pub fn calculate_estimated_byte_size(&self) { + let mut size = core::mem::size_of::(); + + if let Some(store) = self.store() { + size += core::mem::size_of::(); + match &store.data { + store::Data::Bytes(bytes) => { + size += bytes.stored_name.len(); + size += if self.size.get() != MAX_SIZE { + self.size.get() as usize + } else { + bytes.len() as usize + }; + } + store::Data::File(file) => size += file.pathlike.estimated_size(), + store::Data::S3(s3) => size += s3.estimated_size(), + } + } + + let content_type = self.content_type.get(); + if content_type.is_owned() { + size += content_type.as_slice().len(); + } + self.reported_estimated_size + .set(size + self.name.get().byte_slice().len()); + } + + /// Body of the generated `Blob__estimatedSize` thunk (`estimatedSize: true` + /// in response.classes.ts): what `Blob__create` reports when it allocates + /// the wrapper and what `JSBlob::visitChildren` re-reports on every GC. + #[inline] + pub fn estimated_size(&self) -> usize { + self.reported_estimated_size.get() + } + /// Tear down owned resources; if /// heap-allocated, also frees the heap box. pub fn deinit(&mut self) { diff --git a/src/runtime/api/Archive.rs b/src/runtime/api/Archive.rs index 33922cf458e1..cc25ae2e303c 100644 --- a/src/runtime/api/Archive.rs +++ b/src/runtime/api/Archive.rs @@ -11,7 +11,7 @@ use bun_glob as glob; use bun_jsc::{ self as jsc, CallFrame, JSGlobalObject, JSMap, JSPromise, JSPromiseStrong, JSValue, JsResult, }; -use bun_jsc::{StringJsc as _, SysErrorJsc as _}; +use bun_jsc::{JsClass as _, StringJsc as _, SysErrorJsc as _}; use bun_libarchive as libarchive; use bun_sys::{self, Fd, FdDirExt as _, FdExt as _, Mode}; @@ -865,10 +865,7 @@ impl TaskContext for BlobContext { // self.result already replaced with Uncompressed above — ownership transferred Ok(PromiseResult::Resolve(match self.output_type { BlobOutputType::Blob => { - let blob_ptr = - Blob::new(Blob::create_with_bytes_and_allocator(data, global, false)); - // SAFETY: blob_ptr is the heap allocation just produced by Blob::new. - unsafe { (*blob_ptr).to_js(global) } + Blob::create_with_bytes_and_allocator(data, global, false).to_js(global) } BlobOutputType::Bytes => { // Ownership transfers to JSC's `MarkedArrayBuffer_deallocator`. @@ -881,9 +878,7 @@ impl TaskContext for BlobContext { // The clone bumps the refcount; ownership of // the new ref transfers into the Blob via init_with_store. let store = self.store.clone(); - let blob_ptr = Blob::new(Blob::init_with_store(store, global)); - // SAFETY: blob_ptr is the heap allocation just produced by Blob::new. - PromiseResult::Resolve(unsafe { (*blob_ptr).to_js(global) }) + PromiseResult::Resolve(Blob::init_with_store(store, global).to_js(global)) } BlobOutputType::Bytes => { // On allocation failure, reject the promise instead of aborting. @@ -1173,10 +1168,7 @@ impl TaskContext for FilesContext { for entry in entries.iter_mut() { let data = core::mem::take(&mut entry.data); // Ownership transferred - let blob_ptr = - Blob::new(Blob::create_with_bytes_and_allocator(data, global, false)); - // SAFETY: blob_ptr is the heap allocation just produced by Blob::new. - let blob = unsafe { &mut *blob_ptr }; + let blob = Blob::create_with_bytes_and_allocator(data, global, false); blob.is_jsdom_file.set(true); blob.name.set(bun_core::String::clone_utf8(&entry.path)); blob.last_modified.set((entry.mtime * 1000) as f64); diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index fdedb2be588f..269668936666 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -1971,7 +1971,8 @@ fn get_is_standalone_executable(global_this: &JSGlobalObject, _: &JSObject) -> J } fn get_embedded_files(global_this: &JSGlobalObject, _: &JSObject) -> JsResult { - use crate::webcore::blob::{Blob, BlobExt as _}; + use crate::webcore::blob::Blob; + use bun_jsc::JsClass as _; use bun_standalone_graph::{File as GraphFile, Graph as StandaloneModuleGraph}; // SAFETY: bun_vm() returns the live thread-local VM for a Bun-owned global. let vm = global_this.bun_vm(); @@ -2022,11 +2023,9 @@ fn get_embedded_files(global_this: &JSGlobalObject, _: &JSObject) -> JsResult> = const { core::cell::RefCell::new(None) }; @@ -3036,9 +3036,7 @@ mod stdio_stores { // store.ref() — extra +1 for the new Blob. s.as_ref().unwrap().clone() }); - let blob = Blob::new(Blob::init_with_store(store, global_this)); - // SAFETY: `Blob::new` heap-allocates; the JS wrapper takes ownership. - unsafe { (&*blob).to_js(global_this) } + Blob::init_with_store(store, global_this).to_js(global_this) } pub(super) fn stdin(global_this: &JSGlobalObject) -> JSValue { diff --git a/src/runtime/image/Image.rs b/src/runtime/image/Image.rs index 7a02e6a4d7ff..5602534d4340 100644 --- a/src/runtime/image/Image.rs +++ b/src/runtime/image/Image.rs @@ -1854,11 +1854,7 @@ impl PipelineTask { format.mime().as_bytes(), )); blob.content_type_was_set.set(true); - // UFCS to pick the consuming `JsClass::to_js(self, _)` - // (heap-promotes via `Blob::new`) over the inherent - // `Blob::to_js(&mut self, _)` that expects an - // already-heap-allocated receiver. - promise.resolve(global, ::to_js(blob, global))?; + promise.resolve(global, blob.to_js(global))?; } tag @ (Deliver::Base64 | Deliver::DataUrl) => { // This arm copies the bytes out — re-arm `Encoded::drop` so diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index 006cf841e312..809521bfcca6 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -128,8 +128,6 @@ pub use bun_jsc::webcore_types::{Blob, BlobContentType, ClosingState, MAX_SIZE, /// keeps its window's end across structuredClone/postMessage const SERIALIZATION_VERSION: u8 = 4; -pub use bun_jsc::generated::JSBlob as js; - // ────────────────────────────────────────────────────────────────────────── // is_s3: defined once above (near is_bun_file); duplicate removed to fix E0034. @@ -388,9 +386,6 @@ pub trait BlobExt { ) -> JsResult where Self: Sized; - fn calculate_estimated_byte_size(&self); - fn estimated_size(&self) -> usize; - fn to_js(&self, global_object: &JSGlobalObject) -> JSValue; fn find_or_create_file_from_path( path_or_fd: &mut PathOrFileDescriptor, global_this: &JSGlobalObject, @@ -1961,12 +1956,7 @@ impl BlobExt for Blob { blob.content_type_was_set .set(self.content_type_was_set.get() || content_type_was_allocated); - let ptr = Blob::new(blob); - // SAFETY: `ptr` just came from `heap::alloc` in `Blob::new`. Explicit - // `&mut *` forces the inherent `Blob::to_js(&mut self)` (which calls - // `calculate_estimated_byte_size` and routes S3 blobs to - // `S3File.toJSUnchecked`) over the by-value `JsClass::to_js`. - unsafe { BlobExt::to_js(&*ptr, global_this) } + blob.to_js(global_this) } /// https://w3c.github.io/FileAPI/#slice-method-algo @@ -1976,10 +1966,7 @@ impl BlobExt for Blob { let args = &mut arguments_[..]; if self.size.get() == 0 { - let ptr = Blob::new(Blob::init_empty(global_this)); - // SAFETY: `ptr` just came from `heap::alloc` in `Blob::new`; force - // the inherent `Blob::to_js(&mut self)` over `JsClass::to_js`. - return Ok(unsafe { BlobExt::to_js(&*ptr, global_this) }); + return Ok(Blob::init_empty(global_this).to_js(global_this)); } // If the optional start parameter is not used as a parameter, let relativeStart be 0. @@ -2395,7 +2382,6 @@ impl BlobExt for Blob { } } - blob.calculate_estimated_byte_size(); Ok(Blob::new(blob)) } @@ -3537,54 +3523,6 @@ impl BlobExt for Blob { // is_detached: defined once above; duplicate removed to fix E0034. - fn calculate_estimated_byte_size(&self) { - // in-memory size. not the size on disk. - let mut size: usize = core::mem::size_of::(); - - if let Some(store) = self.store.get() { - size += core::mem::size_of::(); - match &store.data { - store::Data::Bytes(bytes) => { - size += bytes.stored_name.len(); - size += if self.size.get() != MAX_SIZE { - self.size.get() as usize - } else { - bytes.len() as usize - }; - } - store::Data::File(file) => size += file.pathlike.estimated_size(), - store::Data::S3(s3) => size += s3.estimated_size(), - } - } - - let ct = self.content_type.get(); - self.reported_estimated_size.set( - size + (ct.as_slice().len() * (ct.is_owned() as usize)) - + self.name.get().byte_slice().len(), - ); - } - - fn estimated_size(&self) -> usize { - self.reported_estimated_size.get() - } - - fn to_js(&self, global_object: &JSGlobalObject) -> JSValue { - // if cfg!(debug_assertions) { debug_assert!(self.is_heap_allocated()); } - self.calculate_estimated_byte_size(); - - // R-2: `&self` receiver, but the FFI shims take `*mut Blob` (the - // heap-allocated `m_ctx` pointer). `self` *is* that allocation (caller - // contract), so the const→mut cast is the original `Blob::new` provenance. - let this = std::ptr::from_ref::(self).cast_mut(); - if self.is_s3() { - // SAFETY: `self` is a heap-allocated *mut Blob (see `Blob::new`); the - // C++ side wraps it in a JSS3File without taking a second ref. - return crate::webcore::s3_file::to_js_unchecked(global_object, this); - } - - js::to_js_unchecked(global_object, this) - } - /// `Bun.file(pathOrFd)` core: wrap a path-or-fd in a `Store::File` and /// return a Blob viewing it. Runtime `check_s3` matches the call shape used /// by `server_body.rs` / `fetch.rs` (collapsed from a const generic since @@ -4075,16 +4013,15 @@ fn on_structured_clone_deserialize>( // Bun.file() keeps its window's end. MAX_SIZE means unknown. let mut file_size: Option = None; - let blob: *mut Blob = match store_tag { - store::SerializeTag::Bytes => 'bytes: { + // `blob` stays a by-value local until the final `to_js`, so an early `?` on + // a truncated record drops it (and releases its store) like any other + // local. + let blob: Blob = match store_tag { + store::SerializeTag::Bytes => { let bytes_len = reader.read_int_le::()?; let bytes = read_slice(reader, bytes_len as usize)?; let blob = Blob::init(bytes, global_this); - // `blob` now owns `bytes` (via its Store when non-empty). If any - // of the remaining reads fail before we heap-promote it, Drop on - // `blob` releases the store so the payload bytes don't leak. - let guard = scopeguard::guard(blob, |mut b| b.deinit()); 'versions: { if version == 1 { @@ -4094,8 +4031,7 @@ fn on_structured_clone_deserialize>( let name_len = reader.read_int_le::()?; let name = read_slice(reader, name_len as usize)?; - // ScopeGuard derefs to its inner Blob. - if let Some(store) = (*guard).store() { + if let Some(store) = blob.store() { if let store::Data::Bytes(bytes_store) = &mut store.data_mut() { // Transfer ownership of the local `name: Vec` into // `stored_name` (a `Box<[u8]>`); freed by `Bytes::Drop`. @@ -4109,10 +4045,9 @@ fn on_structured_clone_deserialize>( } } - let blob = scopeguard::ScopeGuard::into_inner(guard); - break 'bytes Blob::new(blob); + blob } - store::SerializeTag::File => 'file: { + store::SerializeTag::File => { use crate::node::types::PathOrFileDescriptorSerializeTag; if version >= 4 { file_size = Some(reader.read_int_le::()?); @@ -4132,11 +4067,7 @@ fn on_structured_clone_deserialize>( return Err(crate::Error::InvalidValue); } let mut path_or_fd = PathOrFileDescriptor::Fd(fd); - break 'file Blob::new(Blob::find_or_create_file_from_path( - &mut path_or_fd, - global_this, - true, - )); + Blob::find_or_create_file_from_path(&mut path_or_fd, global_this, true) } PathOrFileDescriptorSerializeTag::Path => { let path_len = reader.read_int_le::()?; @@ -4154,25 +4085,12 @@ fn on_structured_clone_deserialize>( let mut dest = PathOrFileDescriptor::Path(node::PathLike::String( bun_ptr::cow_slice::CowSlice::init_owned(path.into_boxed_slice()), )); - break 'file Blob::new(Blob::find_or_create_file_from_path( - &mut dest, - global_this, - true, - )); + Blob::find_or_create_file_from_path(&mut dest, global_this, true) } } } - store::SerializeTag::Empty => Blob::new(Blob::init_empty(global_this)), + store::SerializeTag::Empty => Blob::init_empty(global_this), }; - // `blob` is heap-allocated past this point; on any remaining error - // (truncated trailer fields) tear down both the heap object and its - // store. `content_type` is handled by its own Drop above since it - // hasn't been attached to `blob` yet. - // SAFETY: blob is a freshly-allocated heap pointer from Blob::new. - let blob_guard = scopeguard::guard(blob, |b| unsafe { (*b).deinit() }); - // SAFETY: `blob_guard` holds the sole pointer to the fresh heap allocation. - // Shared access only — Blob state is Cell/JsCell-based. - let blob = unsafe { &**blob_guard }; 'versions: { if version == 1 { @@ -4198,11 +4116,6 @@ fn on_structured_clone_deserialize>( } } - debug_assert!( - blob.is_heap_allocated(), - "expected blob to be heap-allocated" - ); - // `offset` comes from untrusted bytes. Clamp it so a crafted payload cannot // make shared_view() slice past the end of the backing store (OOB heap read). blob.offset.set(offset as SizeType); // intentional truncate @@ -4229,10 +4142,7 @@ fn on_structured_clone_deserialize>( blob.content_type_was_set.set(content_type_was_set); } - let blob_ptr = scopeguard::ScopeGuard::into_inner(blob_guard); - // SAFETY: blob_ptr is valid; toJS is infallible. Explicit `&mut *` forces - // the inherent `Blob::to_js(&mut self)` over `JsClass::to_js(self)`. - Ok(unsafe { BlobExt::to_js(&*blob_ptr, global_this) }) + Ok(blob.to_js(global_this)) } // ────────────────────────────────────────────────────────────────────────── @@ -4737,13 +4647,10 @@ pub(crate) fn write_file_with_source_destination( // If this is bytes <> bytes, we can just duplicate it // this is an edgecase // it will happen if someone did Bun.write(new Blob([123]), new Blob([456])) - let cloned = Blob::new(source_blob.dupe()); - // SAFETY: ptr was just produced by heap::alloc in Blob::new; the - // inherent `to_js(&mut self)` (not the by-value `JsClass` one) hands - // ownership to the C++ wrapper. - return Ok(JSPromise::resolved_promise_value(ctx, unsafe { - BlobExt::to_js(&*cloned, ctx) - })); + return Ok(JSPromise::resolved_promise_value( + ctx, + source_blob.dupe().to_js(ctx), + )); } else if destination_type == store::DataTag::Bytes && (source_type == store::DataTag::File || source_type == store::DataTag::S3) { @@ -5646,12 +5553,9 @@ pub(crate) fn jsdom_file_construct( } // ────────────────────────────────────────────────────────────────────────── -// estimatedSize / constructBunFile / findOrCreateFileFromPath +// constructBunFile / findOrCreateFileFromPath // ────────────────────────────────────────────────────────────────────────── -// `calculate_estimated_byte_size` / `estimated_size`: canonical impls live -// later in this file (near `dupe`/`to_js`). Duplicates removed here. - pub(crate) fn construct_bun_file( global_object: &JSGlobalObject, callframe: &CallFrame, @@ -5712,10 +5616,7 @@ pub(crate) fn construct_bun_file( } } - let ptr = Blob::new(blob); - // SAFETY: ptr was just produced by heap::alloc in Blob::new. Explicit - // `&mut *` forces inherent `Blob::to_js(&mut self)` over `JsClass::to_js(self)`. - Ok(unsafe { BlobExt::to_js(&*ptr, global_object) }) + Ok(blob.to_js(global_object)) } // `find_or_create_file_from_path`: canonical impl lives later in this file @@ -6445,13 +6346,9 @@ impl Any { self.to_uint8_array_transfer(global_this) } streams::BufferActionTag::Blob => { - let result = Blob::new(self.to_blob(global_this)); - // SAFETY: `Blob::new` returns a fresh heap allocation we own; - // `BlobExt::to_js` (the `&mut self` overload) consumes the - // pointer into a JS wrapper which takes ownership. - unsafe { (*result).global_this.set(global_this) }; - // SAFETY: same fresh `result` allocation; ownership transfers to the JS wrapper. - Ok(BlobExt::to_js(unsafe { &*result }, global_this)) + let blob = self.to_blob(global_this); + blob.global_this.set(global_this); + Ok(blob.to_js(global_this)) } streams::BufferActionTag::ArrayBuffer => { if matches!(self, Any::Blob(_)) { diff --git a/src/runtime/webcore/Body.rs b/src/runtime/webcore/Body.rs index 93041722ba08..68a76af3abb7 100644 --- a/src/runtime/webcore/Body.rs +++ b/src/runtime/webcore/Body.rs @@ -20,7 +20,7 @@ use crate::webcore::form_data::AsyncFormDataExt as _; use bun_core::{String as BunString, ZigString}; use bun_core::{WTFStringImpl, WTFStringImplExt as _, WTFStringImplStruct}; use bun_jsc::ZigStringJsc as _; -use bun_jsc::{JsCell, StringJsc as _}; +use bun_jsc::{JsCell, JsClass as _, StringJsc as _}; /// Deref the `Value::WTFStringImpl` / `AnyBlob::WTFStringImpl` payload. /// Centralises the per-site `(**s)` raw deref at the dozen `match` arms below @@ -1181,9 +1181,7 @@ impl Value { result?; } Action::None | Action::GetBlob => { - let blob_ptr = Blob::new(new.use_()); - // SAFETY: `Blob::new` returns a freshly heap-allocated *mut Blob. - let blob = unsafe { &mut *blob_ptr }; + let blob = new.use_(); if let Some(fetch_headers) = headers { // `headers` is a live C++ FetchHeaders handle; // `FetchHeaders` is an opaque ZST FFI handle (S008) — safe deref. @@ -1194,12 +1192,12 @@ impl Value { { let content_slice = content_type.to_slice(); let mime_type = MimeType::init(content_slice.slice(), true, None); - set_blob_content_type(blob, mime_type); + set_blob_content_type(&blob, mime_type); // content_slice dropped (replaces defer content_slice.deinit()) } } if !blob.content_type_was_set.get() && blob.store.get().is_some() { - set_blob_content_type(blob, bun_http_types::MimeType::TEXT); + set_blob_content_type(&blob, bun_http_types::MimeType::TEXT); } promise.resolve(global, blob.to_js(global))?; } @@ -2205,9 +2203,7 @@ pub(crate) trait BodyMixin: BodyOwnerJs + Sized { } let value = self.get_body_value(); - let blob_ptr = Blob::new(value.use_()); - // SAFETY: `Blob::new` returns a freshly heap-allocated, ref-counted Blob. - let blob = unsafe { &mut *blob_ptr }; + let blob = value.use_(); if blob.content_type().is_empty() { if let Some(fetch_headers) = BodyMixin::get_fetch_headers(self) { // `fetch_headers` is a live C++ FetchHeaders handle; @@ -2216,12 +2212,12 @@ pub(crate) trait BodyMixin: BodyOwnerJs + Sized { if let Some(content_type) = fetch_headers.fast_get(HTTPHeaderName::ContentType) { let content_slice = content_type.to_slice(); let mime_type = MimeType::init(content_slice.slice(), true, None); - set_blob_content_type(blob, mime_type); + set_blob_content_type(&blob, mime_type); // content_slice dropped (replaces defer content_slice.deinit()) } } if !blob.content_type_was_set.get() && blob.store.get().is_some() { - set_blob_content_type(blob, bun_http_types::MimeType::TEXT); + set_blob_content_type(&blob, bun_http_types::MimeType::TEXT); } } Ok(JSPromise::resolved_promise_value( diff --git a/src/runtime/webcore/ObjectURLRegistry.rs b/src/runtime/webcore/ObjectURLRegistry.rs index 7d28e518fd43..8d1ab6799d15 100644 --- a/src/runtime/webcore/ObjectURLRegistry.rs +++ b/src/runtime/webcore/ObjectURLRegistry.rs @@ -3,11 +3,10 @@ use std::sync::OnceLock; use bun_collections::HashMap; use bun_core::strings; use bun_jsc::virtual_machine::VirtualMachine; -use bun_jsc::{CallFrame, JSGlobalObject, JSValue, JsResult, StringJsc as _, UUID}; +use bun_jsc::{CallFrame, JSGlobalObject, JSValue, JsClass as _, JsResult, StringJsc as _, UUID}; use bun_threading::Guarded; use crate::webcore::Blob; -use crate::webcore::BlobExt as _; // The map is wrapped in a `Guarded` (mutex + value). // @@ -78,9 +77,7 @@ impl ObjectURLRegistry { pathname: &[u8], global_object: &JSGlobalObject, ) -> Option { - let blob = Blob::new(self.resolve_and_dupe(pathname)?); - // SAFETY: `Blob::new` returns a freshly-boxed heap pointer. - Some(unsafe { (*blob).to_js(global_object) }) + Some(self.resolve_and_dupe(pathname)?.to_js(global_object)) } pub(crate) fn revoke(&self, pathname: &[u8]) { diff --git a/src/runtime/webcore/S3Client.rs b/src/runtime/webcore/S3Client.rs index c6a794990b82..2daa19beda48 100644 --- a/src/runtime/webcore/S3Client.rs +++ b/src/runtime/webcore/S3Client.rs @@ -2,11 +2,12 @@ use bstr::BStr; use crate::node::PathLike; use crate::node::types::PathLikeExt as _; -use crate::webcore::blob::BlobExt as _; use crate::webcore::blob::store::S3Ext as _; use crate::webcore::s3::MultiPartUploadOptions; use crate::webcore::s3::client::{ACL, S3Credentials, StorageClass}; -use bun_jsc::{CallFrame, ConsoleFormatter, ErrorCode, JSGlobalObject, JSValue, JsResult}; +use bun_jsc::{ + CallFrame, ConsoleFormatter, ErrorCode, JSGlobalObject, JSValue, JsClass as _, JsResult, +}; use super::s3_file as S3File; @@ -420,16 +421,7 @@ impl S3Client { } }; let options = args.next_eat(); - // `Blob::new` heap-promotes and marks `ref_count = 1` so - // the JSS3File wrapper's `finalize` knows to free the blob. - let blob = crate::webcore::blob::Blob::new(ptr.construct_blob(global, path, options)?); - // `to_js` runs `calculateEstimatedByteSize()` - // before wrapping the heap Blob in a JSS3File so JSC sees the correct - // GC pressure. Route through `BlobExt::to_js` (the `&mut self` method - // that owns the heap pointer), same as `S3File::construct_internal_js`. - // SAFETY: `blob` is a freshly leaked `*mut Blob` from `Blob::new`; - // `to_js` hands ownership of that pointer to the C++ wrapper. - Ok(unsafe { &mut *blob }.to_js(global)) + Ok(ptr.construct_blob(global, path, options)?.to_js(global)) } #[bun_jsc::host_fn(method)] diff --git a/src/runtime/webcore/S3File.rs b/src/runtime/webcore/S3File.rs index d5b2fb577ead..aca13d513743 100644 --- a/src/runtime/webcore/S3File.rs +++ b/src/runtime/webcore/S3File.rs @@ -344,16 +344,6 @@ fn finish_s3_blob( Ok(blob) } -fn construct_s3_file_internal( - global: &JSGlobalObject, - path: PathLike, - options: Option, -) -> JsResult<*mut Blob> { - Ok(Blob::new(construct_s3_file_internal_store( - global, path, options, - )?)) -} - pub(crate) struct S3BlobStatTask { promise: bun_jsc::JSPromiseStrong, // LIFETIMES.tsv: JSC_BORROW (&JSGlobalObject). `BackRef` so the heap task @@ -706,18 +696,7 @@ pub(crate) fn construct_internal_js( path: PathLike, options: Option, ) -> JsResult { - let blob = construct_s3_file_internal(global, path, options)?; - // SAFETY: `blob` is a freshly heap-allocated `*mut Blob` from `Blob::new`. - // Call the `BlobExt::to_js` `&mut self` method (not the by-value - // `JsClass::to_js`), which hands the existing heap pointer to the C++ - // wrapper. - Ok(BlobExt::to_js(unsafe { &mut *blob }, global)) -} - -pub(crate) fn to_js_unchecked(global: &JSGlobalObject, this: *mut Blob) -> JSValue { - // C++ adopts `this` opaquely (stored as `void* m_ctx` in the JS wrapper); - // ownership-transfer contract lives on `to_js_unchecked`'s callers. - BUN__createJSS3FileUnsafely(global, this.cast::()) + Ok(construct_s3_file_internal_store(global, path, options)?.to_js(global)) } // Symbols exported with C linkage and JSC calling convention. @@ -763,15 +742,3 @@ pub(crate) mod exports { bun_jsc::to_js_host_call(global, || super::get_stat(this, global, callframe)) } } - -// C++ side defines `SYSV_ABI EncodedJSValue` (JSS3File.cpp). -bun_jsc::jsc_abi_extern! { - // `&JSGlobalObject` discharges the only deref'd-param precondition; `blob` - // is stored opaquely as `void* m_ctx` (module-private — sole caller is - // `to_js_unchecked`, whose own signature carries the ownership-transfer - // contract). Matches the `*__createObject` precedent. - safe fn BUN__createJSS3FileUnsafely( - global: &JSGlobalObject, - blob: *mut core::ffi::c_void, - ) -> JSValue; -} diff --git a/test/js/web/fetch/blob.test.ts b/test/js/web/fetch/blob.test.ts index fc9b4500bcfc..a41bfd9482d6 100644 --- a/test/js/web/fetch/blob.test.ts +++ b/test/js/web/fetch/blob.test.ts @@ -1,6 +1,7 @@ +import { estimateShallowMemoryUsageOf } from "bun:jsc"; import { describe, expect, test } from "bun:test"; import { bunEnv, bunExe, isASAN, tempDir } from "harness"; -import type { BlobOptions } from "node:buffer"; +import { resolveObjectURL, type BlobOptions } from "node:buffer"; import type { BinaryLike } from "node:crypto"; import path from "node:path"; @@ -735,3 +736,136 @@ describe("Blob from ArrayBuffer-like values", () => { expect(await blob.text()).toBe("abcdefgh"); }); }); + +// Every Blob wrapper, whichever native producer made it, reports the blob's +// in-memory footprint to the GC (`estimateShallowMemoryUsageOf` reports that +// same number), and S3-backed blobs get the S3File wrapper. A producer that +// skips the bookkeeping shows up here as a wrapper whose estimate is just the +// JS cell (a few dozen bytes), which is what `Bun.Image#blob()`, `new File()`, +// parsed `formData()` entries and WebSocket blob messages used to report. +describe("blobs handed to JS report their bytes to the GC", () => { + // Incompressible payload so gzip and PNG outputs stay far above the fixed + // per-blob overhead, which keeps `estimate >= size` a meaningful bound. + const noise = new Uint8Array(64 * 1024); + for (let i = 0, seed = 0x2545f491; i < noise.length; i++) { + seed = (Math.imul(seed, 1103515245) + 12345) >>> 0; + noise[i] = seed >>> 24; + } + + // 64x64 24-bit BMP (54-byte BITMAPINFOHEADER file, uncompressed) whose pixel + // rows are the noise above; the row stride (192 bytes) is already 4-aligned. + function noiseBmp(): Uint8Array { + const pixelBytes = 64 * 64 * 3; + const bmp = new Uint8Array(54 + pixelBytes); + const header = new DataView(bmp.buffer); + bmp.set([0x42, 0x4d]); // "BM" + header.setUint32(2, bmp.length, true); + header.setUint32(10, 54, true); // pixel array offset + header.setUint32(14, 40, true); // BITMAPINFOHEADER size + header.setInt32(18, 64, true); // width + header.setInt32(22, 64, true); // height + header.setUint16(26, 1, true); // planes + header.setUint16(28, 24, true); // bits per pixel + header.setUint32(34, pixelBytes, true); + bmp.set(noise.subarray(0, pixelBytes), 54); + return bmp; + } + + const producers: Record Blob | Promise> = { + "new Blob()": () => new Blob([noise]), + "new File()": () => new File([noise], "noise.bin"), + "Blob#slice()": () => new Blob([noise]).slice(0, 48 * 1024), + "structuredClone(blob)": () => structuredClone(new Blob([noise])), + "resolveObjectURL()": () => { + const url = URL.createObjectURL(new Blob([noise])); + try { + return resolveObjectURL(url)!; + } finally { + URL.revokeObjectURL(url); + } + }, + "ReadableStream#blob() on Blob#stream()": () => new Blob([noise]).stream().blob(), + "Response#blob() with a buffered body": () => new Response(noise).blob(), + "Response#blob() with a streamed body": () => new Response(new Blob([noise]).stream()).blob(), + "Request#blob()": () => new Request("http://localhost/", { method: "POST", body: noise }).blob(), + "Response#formData() file entry": async () => { + const form = new FormData(); + form.append("f", new Blob([noise]), "noise.bin"); + return (await new Response(form).formData()).get("f") as Blob; + }, + 'WebSocket message with binaryType = "blob"': async () => { + await using server = Bun.serve({ + port: 0, + fetch: (request, server) => (server.upgrade(request) ? undefined : new Response(null, { status: 400 })), + websocket: { + open(ws) { + ws.send(noise); + }, + message() {}, + }, + }); + const ws = new WebSocket(server.url); + ws.binaryType = "blob"; + const message = Promise.withResolvers(); + ws.onmessage = event => message.resolve(event.data); + ws.onerror = () => message.reject(new Error("WebSocket connection failed")); + ws.onclose = event => message.reject(new Error(`WebSocket closed (${event.code}) before delivering the message`)); + const blob = await message.promise; + const closed = Promise.withResolvers(); + ws.onclose = () => closed.resolve(); + ws.close(); + await closed.promise; + return blob; + }, + "Bun.Archive#blob()": () => new Bun.Archive({ "noise.bin": noise }).blob(), + "Bun.Archive#blob() with gzip": () => new Bun.Archive({ "noise.bin": noise }, { compress: "gzip" }).blob(), + "Bun.Archive#files()": async () => { + const tar = await new Bun.Archive({ "noise.bin": noise }).bytes(); + return (await new Bun.Archive(tar).files()).get("noise.bin")!; + }, + "Bun.Image#blob()": () => new Bun.Image(noiseBmp()).png().blob(), + }; + + for (const [name, produce] of Object.entries(producers)) { + test(name, async () => { + const blob = await produce(); + expect(blob).toBeInstanceOf(Blob); + // Sanity check on the producer itself: the payload has to dwarf the + // per-blob overhead for the bound below to say anything. + expect(blob.size).toBeGreaterThan(8 * 1024); + expect(estimateShallowMemoryUsageOf(blob)).toBeGreaterThanOrEqual(blob.size); + }); + } + + const credentials = { accessKeyId: "key", secretAccessKey: "secret", bucket: "bucket" }; + const s3Producers = { + "Bun.s3.file()": () => Bun.s3.file("object.bin", credentials), + "new Bun.S3Client().file()": () => new Bun.S3Client(credentials).file("object.bin"), + "Bun.S3Client.file()": () => Bun.S3Client.file("object.bin", credentials), + }; + + // File- and S3-backed blobs keep no bytes in memory, so what is observable is + // the bookkeeping itself: the wrapper accounts for its store (and for S3, its + // credentials) on top of what an empty in-memory blob reports. + const storeBackedProducers: Record Blob> = { + "Bun.file()": () => Bun.file(import.meta.path), + "Bun.stdin": () => Bun.stdin, + ...s3Producers, + }; + + for (const [name, produce] of Object.entries(storeBackedProducers)) { + test(name, () => { + const blob = produce(); + expect(blob).toBeInstanceOf(Blob); + expect(estimateShallowMemoryUsageOf(blob)).toBeGreaterThan(estimateShallowMemoryUsageOf(new Blob([]))); + }); + } + + test("only S3-backed blobs get the S3File wrapper", () => { + for (const produce of Object.values(s3Producers)) { + expect(typeof produce().presign).toBe("function"); + } + expect("presign" in Bun.file(import.meta.path)).toBe(false); + expect("presign" in new Blob([noise])).toBe(false); + }); +});