Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 45 additions & 40 deletions src/runtime/webcore/Blob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2371,8 +2371,7 @@
}
}

blob.calculate_estimated_byte_size();
Ok(Blob::new(blob))
Ok(new_for_bindings(blob))
}

// `finalize` is inherent on `Blob` (bun_jsc::webcore_types) so codegen's
Expand Down Expand Up @@ -4255,9 +4254,18 @@
}
}

/// Heap-promotes a Blob whose wrapper the bindings build from the returned pointer
/// (the generated constructor, `Blob__create`, `WebCore::Blob`) rather than via
/// `BlobExt::to_js`. `Blob__estimatedSize` only reads the cached size (it also
/// runs on the GC thread), so it is computed here, once every field is final.
Comment thread
robobun marked this conversation as resolved.
Resolved
fn new_for_bindings(blob: Blob) -> *mut Blob {
blob.calculate_estimated_byte_size();
Blob::new(blob)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

#[unsafe(no_mangle)]
pub(crate) extern "C" fn Blob__dupe(this: &Blob) -> *mut Blob {
Blob::new(this.dupe_with_content_type(true))
new_for_bindings(this.dupe_with_content_type(true))
}

#[unsafe(no_mangle)]
Expand Down Expand Up @@ -5995,6 +6003,29 @@
unsafe { (*blob).shared_view().len() }
}

/// # Safety
/// `[ptr, ptr+len)` must be a valid readable byte range (or `ptr` null / `len` 0).
Comment thread
robobun marked this conversation as resolved.
Resolved
unsafe fn blob_from_bytes(global_this: &JSGlobalObject, ptr: *const u8, len: usize) -> Blob {
if ptr.is_null() || len == 0 {
return Blob::init_empty(global_this);
}
// SAFETY: caller guarantees [ptr, ptr+len) is valid.
let bytes = unsafe { bun_core::ffi::slice(ptr, len) }.to_vec();
Blob::init_with_store(Store::init(bytes), global_this)
}

/// # Safety
/// `mime` must be a NUL-terminated C string.
Comment thread
robobun marked this conversation as resolved.
Resolved
unsafe fn set_content_type_from_cstr(blob: &Blob, mime: *const c_char) {
// SAFETY: forwarded from caller's contract.
let mime_slice = unsafe { bun_core::ffi::cstr(mime) }.to_bytes();
if !mime_slice.is_empty() {
blob.content_type
.set(BlobContentType::Owned(mime_slice.into()));
blob.content_type_was_set.set(true);
}
}

/// # Safety
/// `[ptr, ptr+len)` must be a valid readable byte range (or `ptr` null / `len` 0).
#[unsafe(no_mangle)]
Expand All @@ -6003,22 +6034,15 @@
ptr: *const u8,
len: usize,
) -> *mut Blob {
if ptr.is_null() || len == 0 {
return Blob::new(Blob::init_empty(global_this));
}
// SAFETY: caller guarantees [ptr, ptr+len) is valid.
let bytes = unsafe { bun_core::ffi::slice(ptr, len) }.to_vec();
let store = Store::init(bytes);
Blob::new(Blob::init_with_store(store, global_this))
// SAFETY: forwarded from caller's contract.
new_for_bindings(unsafe { blob_from_bytes(global_this, ptr, len) })
}

/// Same as Blob__fromBytes but stamps content_type. `mime` must be a
/// string literal with process lifetime (not freed by deinit — the caller
/// passes one of the image/* constants).
/// Same as Blob__fromBytes but stamps content_type (copied out of `mime`).
///
/// # Safety
/// `[ptr, ptr+len)` must be a valid readable byte range and `mime` a
/// NUL-terminated `'static` C string.
/// NUL-terminated C string.
#[unsafe(no_mangle)]
pub(crate) unsafe extern "C" fn Blob__fromBytesWithType(
global_this: &JSGlobalObject,
Expand All @@ -6027,20 +6051,10 @@
mime: *const c_char,
) -> *mut Blob {
// SAFETY: forwarded from caller's contract.
let blob = unsafe { Blob__fromBytes(global_this, ptr, len) };
// SAFETY: caller guarantees `mime` is a NUL-terminated 'static C string.
let mime_slice = unsafe { bun_core::ffi::cstr(mime) }.to_bytes();
if !mime_slice.is_empty() {
// SAFETY: `blob` is a fresh heap allocation returned by `Blob__fromBytes`;
// we are the sole owner until this function returns it.
unsafe {
(*blob)
.content_type
.set(BlobContentType::Owned(mime_slice.into()));
(*blob).content_type_was_set.set(true);
}
}
blob
let blob = unsafe { blob_from_bytes(global_this, ptr, len) };
// SAFETY: forwarded from caller's contract.
unsafe { set_content_type_from_cstr(&blob, mime) };
new_for_bindings(blob)
}

/// Adopts an mmap'd region — no copy. The Blob's store holds the mapping;
Expand Down Expand Up @@ -6069,19 +6083,10 @@
// SAFETY: caller (C++ WebKit screenshot path) guarantees `[ptr, ptr+len)`
// is a valid page-aligned mmap'd region we now own.
let store = Store::init_mmap(unsafe { core::slice::from_raw_parts_mut(ptr, len) });
let blob = Blob::new(Blob::init_with_store(store, global_this));
let blob = Blob::init_with_store(store, global_this);
// SAFETY: caller (C++) passes a valid NUL-terminated C string.
let mime_slice = unsafe { bun_core::ffi::cstr(mime) }.to_bytes();
if !mime_slice.is_empty() {
// SAFETY: `blob` was just produced by heap::alloc in Blob::new.
unsafe {
(*blob)
.content_type
.set(BlobContentType::Owned(mime_slice.into()));
(*blob).content_type_was_set.set(true);
}
}
blob
unsafe { set_content_type_from_cstr(&blob, mime) };

Check warning on line 6088 in src/runtime/webcore/Blob.rs

View check run for this annotation

Claude / Claude Code Review

Blob__fromMmapWithType safety doc still requires 'static mime

The PR drops the stale `'static` lifetime requirement from `Blob__fromBytesWithType`'s safety doc (mime is copied into an owned `BlobContentType`), but the sibling `Blob__fromMmapWithType` — which now routes through the same `set_content_type_from_cstr` helper — still documents at lines 6066-6067 that `mime` must be a "NUL-terminated `'static` C string". Drop `'static` here too to match.
Comment thread
robobun marked this conversation as resolved.
Resolved
new_for_bindings(blob)
}
}

Expand Down
76 changes: 76 additions & 0 deletions test/js/bun/util/heap-snapshot.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,82 @@ describe("Native types report their size correctly", () => {

delete globalThis.ws;
});

// These Blobs are created by the native bindings rather than by `new Blob()`,
// so each of those constructors has to record the size on its own.
describe("Blob handed to JS by the native bindings", () => {
const payloadSize = 1024 * 1024;

function multipartUpload() {
const upload = new FormData();
upload.append("file", new Blob([Buffer.alloc(payloadSize, "abc")]), "upload.bin");
return upload;
}

it.each(["Request", "Response"] as const)("File parsed from a multipart %s body", async bodyOwner => {
const parsed =
bodyOwner === "Request"
? await new Request("http://example.com/", { method: "POST", body: multipartUpload() }).formData()
: await new Response(multipartUpload()).formData();

const file = parsed.get("file") as File;
expect(file.size).toBe(payloadSize);
expect(estimateShallowMemoryUsageOf(file)).toBeGreaterThan(payloadSize);
expect(estimateShallowMemoryUsageOf(file)).toBeLessThan(payloadSize * 2);

// FormData's own size goes through the entry it holds, not through the JS File.
expect(estimateShallowMemoryUsageOf(parsed)).toBeGreaterThan(payloadSize);
expect(estimateShallowMemoryUsageOf(parsed)).toBeLessThan(payloadSize * 2);
});

it("parsed multipart File appended to another FormData", async () => {
const parsed = await new Response(multipartUpload()).formData();

const forwarded = new FormData();
const empty = estimateShallowMemoryUsageOf(forwarded);
forwarded.append("file", parsed.get("file") as File);
expect(estimateShallowMemoryUsageOf(forwarded)).toBeGreaterThan(empty + payloadSize);
expect(estimateShallowMemoryUsageOf(forwarded)).toBeLessThan(empty + payloadSize * 2);
});

it('WebSocket message received with binaryType = "blob"', async () => {
using server = Bun.serve({
port: 0,
fetch(req, server) {
if (server.upgrade(req)) return;
return new Response("expected a websocket upgrade", { status: 400 });
},
websocket: {
open(ws) {
ws.sendBinary(new Uint8Array(payloadSize));
},
message() {},
},
});

const ws = new WebSocket(server.url);
ws.binaryType = "blob";
const { promise, resolve, reject } = Promise.withResolvers<MessageEvent>();
ws.onmessage = resolve;
ws.onerror = reject;
ws.onclose = event => reject(new Error(`WebSocket closed before a message arrived (code ${event.code})`));
try {
const event = await promise;
const blob = event.data as Blob;
expect(blob).toBeInstanceOf(Blob);
expect(blob.size).toBe(payloadSize);
expect(estimateShallowMemoryUsageOf(blob)).toBeGreaterThan(payloadSize);
expect(estimateShallowMemoryUsageOf(blob)).toBeLessThan(payloadSize * 2);

// MessageEvent reports the Blob it holds as its own cost.
expect(estimateShallowMemoryUsageOf(event)).toBeGreaterThan(payloadSize);
expect(estimateShallowMemoryUsageOf(event)).toBeLessThan(payloadSize * 2);
} finally {
ws.onclose = null;
ws.close();
}
});
});
});

describe("CommonJS Module cached slots are visible in heap snapshots", () => {
Expand Down
3 changes: 3 additions & 0 deletions test/js/bun/webview/webview-chrome.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { estimateShallowMemoryUsageOf } from "bun:jsc";
import { expect, test } from "bun:test";
import { bunEnv, bunExe, isCI, isMacOS, isMacOSVersionAtLeast } from "harness";

Expand Down Expand Up @@ -178,6 +179,8 @@ it("chrome: screenshot returns a PNG Blob", async () => {
expect(bytes[3]).toBe(0x47);
// Bun.write accepts the Blob directly — the MIME type carries through.
expect(blob.size).toBeGreaterThan(100);
// The natively created Blob has to report its bytes to the GC like a Blob built from them.
expect(estimateShallowMemoryUsageOf(blob)).toBeGreaterThanOrEqual(estimateShallowMemoryUsageOf(new Blob([bytes])));
});

it("chrome: screenshot format options produce the right magic bytes", async () => {
Expand Down
3 changes: 3 additions & 0 deletions test/js/bun/webview/webview.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { dlopen, FFIType, ptr, toArrayBuffer } from "bun:ffi";
import { estimateShallowMemoryUsageOf } from "bun:jsc";
import { expect, test } from "bun:test";
import { bunEnv, bunExe, isCI, isMacOS, isMacOSVersionAtLeast, tempDir } from "harness";

Expand Down Expand Up @@ -369,6 +370,8 @@ it("screenshot returns a PNG Blob", async () => {
expect(bytes[1]).toBe(0x50);
expect(bytes[2]).toBe(0x4e);
expect(bytes[3]).toBe(0x47);
// The mmap-backed Blob has to report its bytes to the GC like a Blob built from them.
expect(estimateShallowMemoryUsageOf(blob)).toBeGreaterThanOrEqual(estimateShallowMemoryUsageOf(new Blob([bytes])));
});

it("screenshot format options", async () => {
Expand Down