Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
28 changes: 23 additions & 5 deletions src/jsc/array_buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,27 @@ impl ArrayBuffer {
}
}

/// True for a `SharedArrayBuffer`, which another thread writes during the call, and for a resizable buffer, whose `resize()` unmaps pages as soon as the call runs JS.
pub fn can_change_under_borrow(&self) -> bool {
self.shared || self.resizable
}

/// A private copy of the bytes, made out of line so that no caller is compiled to read a `SharedArrayBuffer` source twice. `None` if it cannot be allocated.
#[inline(never)]
pub fn try_copy_bytes(&self) -> Option<Vec<u8>> {
let mut copy = Vec::new();
if self.is_detached() {
return Some(copy);
}
copy.try_reserve_exact(self.byte_len).ok()?;
// SAFETY: `ptr` is non-null and valid for `byte_len` bytes, and `copy` has that capacity. The read stays on the raw pointer: another thread can write a `SharedArrayBuffer` during it, so no `&[u8]` may cover those bytes.
unsafe {
core::ptr::copy_nonoverlapping(self.ptr, copy.as_mut_ptr(), self.byte_len);
copy.set_len(self.byte_len);
}
Some(copy)
}

// require('buffer').kMaxLength.
// keep in sync with Bun::Buffer::kMaxLength
pub const MAX_SIZE: c_uint = c_uint::MAX;
Expand Down Expand Up @@ -700,12 +721,9 @@ impl PinnedArrayBuffer {
{
return true;
}
let bytes = self.buffer.byte_slice();
let mut copy = Vec::new();
if copy.try_reserve_exact(bytes.len()).is_err() {
let Some(mut copy) = self.buffer.try_copy_bytes() else {
return false;
}
copy.extend_from_slice(bytes);
};
global.vm().report_extra_memory(copy.len());
self.buffer.ptr = copy.as_mut_ptr();
self.copy = Some(copy);
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ fn with_text_format_source_encoded<R>(
let mut encoding = SourceEncoding::Utf8Text;
let bytes: &[u8] = 'bytes: {
if blob_or_buffer_input == BlobOrBufferInput::Bytes && !input_value.is_string() {
if let Some(v) = BlobOrStringOrBuffer::from_js(global, input_value)? {
if let Some(v) = BlobOrStringOrBuffer::from_js_stable(global, input_value)? {
_blob_hold = v;
encoding = SourceEncoding::Bytes;
break 'bytes _blob_hold.slice();
Expand Down
30 changes: 26 additions & 4 deletions src/runtime/api/JSTranspiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,26 @@ fn level_from_js(global: &JSGlobalObject, value: JSValue) -> JsResult<Option<bun
bun_ast::Level::MAP.from_js(global, value)
}

/// The `code` of `scan` and `transformSync`, with a buffer copied: a macro runs in the middle of the parse and can write, shrink or detach it. A fixed, unshared source over [`bun_ast::Source::MAX_PARSEABLE_LEN`] stays borrowed: the parser rejects it before a macro can run.
fn code_from_js(
global: &JSGlobalObject,
value: JSValue,
) -> JsResult<Option<StringOrBuffer<'static>>> {
let parsed = StringOrBuffer::from_js(global, value)?;
let Some(StringOrBuffer::Buffer(buffer)) = &parsed else {
return Ok(parsed);
};
if !buffer.buffer.can_change_under_borrow()
&& buffer.buffer.byte_len > bun_ast::Source::MAX_PARSEABLE_LEN
{
return Ok(parsed);
}
match buffer.buffer.try_copy_bytes() {
Comment thread
robobun marked this conversation as resolved.
Some(copy) => Ok(Some(StringOrBuffer::owned(copy))),
None => Err(global.throw_out_of_memory()),
}
}

/// Deep-clone a [`MacroMap`]. The keys are `Box<[u8]>`, so an owned copy
/// is needed wherever the map is assigned by value.
fn clone_macro_map(src: &MacroMap) -> MacroMap {
Expand Down Expand Up @@ -1270,7 +1290,7 @@ impl JSTranspiler {
return Err(global.throw_invalid_argument_type("scan", "code", "string or Uint8Array"));
};

let Some(code_holder) = StringOrBuffer::from_js(global, code_arg)? else {
let Some(code_holder) = code_from_js(global, code_arg)? else {
return Err(global.throw_invalid_argument_type("scan", "code", "string or Uint8Array"));
};
let code = code_holder.slice();
Expand Down Expand Up @@ -1359,7 +1379,9 @@ impl JSTranspiler {
};

let code = if let Some(buffer) = code_arg.as_array_buffer(global) {
let bytes = buffer.byte_slice().to_vec();
let Some(bytes) = buffer.try_copy_bytes() else {
return Err(global.throw_out_of_memory());
};
global.vm().report_extra_memory(bytes.len());
StringOrBuffer::owned_isolated(bytes)
} else if let Some(code) = StringOrBuffer::from_js_async(global, code_arg)? {
Expand Down Expand Up @@ -1412,7 +1434,7 @@ impl JSTranspiler {
};

let arena = Arena::new();
let Some(code_holder) = StringOrBuffer::from_js(global, code_arg)? else {
let Some(code_holder) = code_from_js(global, code_arg)? else {
return Err(global.throw_invalid_argument_type(
"transformSync",
"code",
Expand Down Expand Up @@ -1615,7 +1637,7 @@ impl JSTranspiler {
));
};

let Some(code_holder) = StringOrBuffer::from_js(global, code_arg)? else {
let Some(code_holder) = StringOrBuffer::from_js_stable(global, code_arg)? else {
return Err(global.throw_invalid_argument_type(
"scanImports",
"code",
Expand Down
8 changes: 4 additions & 4 deletions src/runtime/api/MarkdownObject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ pub(crate) fn render_to_ansi(
.throw_invalid_arguments(format_args!("Expected a string or buffer to render")));
}

let Some(buffer) = StringOrBuffer::from_js(global_this, input_value)? else {
let Some(buffer) = StringOrBuffer::from_js_stable(global_this, input_value)? else {
return Err(global_this
.throw_invalid_arguments(format_args!("Expected a string or buffer to render")));
};
Expand Down Expand Up @@ -191,7 +191,7 @@ fn render_to_html(global_this: &JSGlobalObject, callframe: &CallFrame) -> JsResu
.throw_invalid_arguments(format_args!("Expected a string or buffer to render")));
}

let Some(buffer) = StringOrBuffer::from_js(global_this, input_value)? else {
let Some(buffer) = StringOrBuffer::from_js_stable(global_this, input_value)? else {
return Err(global_this
.throw_invalid_arguments(format_args!("Expected a string or buffer to render")));
};
Expand Down Expand Up @@ -294,7 +294,7 @@ fn render(global_this: &JSGlobalObject, callframe: &CallFrame) -> JsResult<JSVal
.throw_invalid_arguments(format_args!("Expected a string or buffer to render")));
}

let Some(buffer) = StringOrBuffer::from_js(global_this, input_value)? else {
let Some(buffer) = StringOrBuffer::from_js_stable(global_this, input_value)? else {
return Err(global_this
.throw_invalid_arguments(format_args!("Expected a string or buffer to render")));
};
Comment thread
robobun marked this conversation as resolved.
Expand Down Expand Up @@ -387,7 +387,7 @@ fn render_ast(
.throw_invalid_arguments(format_args!("Expected a string or buffer to render")));
}

let Some(buffer) = StringOrBuffer::from_js(global_this, input_value)? else {
let Some(buffer) = StringOrBuffer::from_js_stable(global_this, input_value)? else {
return Err(global_this
.throw_invalid_arguments(format_args!("Expected a string or buffer to render")));
};
Expand Down
55 changes: 50 additions & 5 deletions src/runtime/node/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,20 @@ impl BlobOrStringOrBuffer {
Self::from_js_maybe_file(global, value, FileBlobs::Reject)
}

/// [`from_js`](Self::from_js), with a buffer copied if [`StringOrBuffer::copy_if_can_change`] says so. A `Blob` owns its bytes.
pub(crate) fn from_js_stable(
global: &JSGlobalObject,
value: JSValue,
) -> JsResult<Option<BlobOrStringOrBuffer>> {
let mut parsed = Self::from_js(global, value)?;
if let Some(Self::StringOrBuffer(input)) = &mut parsed
&& !input.copy_if_can_change()
{
return Err(global.throw_out_of_memory());
}
Ok(parsed)
}

/// Like [`from_js_with_encoding_value_allow_request_response`] but takes an
/// already-parsed [`Encoding`], so callers that must inspect the encoding
/// first (e.g. to validate odd-length hex) don't coerce `encoding_value`
Expand Down Expand Up @@ -319,6 +333,22 @@ impl<'a> StringOrBuffer<'a> {
}

impl StringOrBuffer<'_> {
/// Replaces a borrowed buffer that [`can_change_under_borrow`](jsc::ArrayBuffer::can_change_under_borrow) with a private copy of its bytes. `false` if the copy cannot be allocated.
#[must_use]
pub(crate) fn copy_if_can_change(&mut self) -> bool {
let Self::Buffer(buffer) = self else {
return true;
};
if !buffer.buffer.can_change_under_borrow() {
return true;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
let Some(copy) = buffer.buffer.try_copy_bytes() else {
return false;
};
*self = Self::owned(copy);
true
}

pub(crate) fn into_js(self, ctx: &JSGlobalObject) -> JsResult<JSValue> {
match self {
Self::ThreadIsolatedString(str) | Self::String(str) => str.into_js(ctx),
Expand Down Expand Up @@ -366,11 +396,11 @@ impl StringOrBuffer<'static> {
value: JSValue,
) -> JsResult<Vec<u8>> {
if let Some(array_buffer) = value.as_array_buffer(global_object) {
let bytes = array_buffer.byte_slice();
global_object
.vm()
.report_extra_memory(array_buffer.len as usize);
return Ok(bytes.to_vec());
let Some(bytes) = array_buffer.try_copy_bytes() else {
return Err(global_object.throw_out_of_memory());
};
global_object.vm().report_extra_memory(bytes.len());
return Ok(bytes);
}

let str = bun_core::String::from_js(value, global_object)?;
Expand Down Expand Up @@ -458,6 +488,21 @@ impl StringOrBuffer<'static> {
Self::from_js_maybe_async(global, value, Flavor::Sync, StringObjects::Allow)
}

/// [`from_js`](Self::from_js), with a buffer copied if [`copy_if_can_change`](Self::copy_if_can_change) says so.
#[inline]
pub(crate) fn from_js_stable(
global: &JSGlobalObject,
value: JSValue,
) -> JsResult<Option<Self>> {
let mut parsed = Self::from_js(global, value)?;
if let Some(input) = &mut parsed
&& !input.copy_if_can_change()
{
return Err(global.throw_out_of_memory());
}
Ok(parsed)
}

/// [`from_js`](Self::from_js) for a work-pool job that reads the bytes itself: strings thread-isolated, buffers pinned and GC-rooted, a resizable buffer copied ([`PinnedArrayBuffer::copy_if_resizable`]).
#[inline]
pub(crate) fn from_js_async(
Expand Down
6 changes: 4 additions & 2 deletions src/runtime/webcore/TextDecoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,8 +301,10 @@ impl TextDecoder {

if let Some(ab) = arguments[0].as_array_buffer(global_this) {
array_buffer = ab;
if array_buffer.shared || array_buffer.resizable {
owned_input = Box::<[u8]>::from(array_buffer.slice());
if array_buffer.can_change_under_borrow() {
owned_input = array_buffer
.try_copy_bytes()
.ok_or_else(|| global_this.throw_out_of_memory())?;
break 'input_slice &owned_input;
}
break 'input_slice array_buffer.slice();
Expand Down
68 changes: 68 additions & 0 deletions test/js/bun/md/md-render-callback.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -426,4 +426,72 @@ describe("Bun.markdown buffer input", () => {
input.buffer.transfer();
expect((input.buffer as ArrayBuffer).detached).toBe(true);
});

// A pin stops a detach but not a shrink: `resize()` gives the trimmed pages
// back. Option getters and render callbacks run after the call took its
// input, so a resizable input renders from the bytes that were passed in.
describe("a resizable input that shrinks while the call runs", () => {
const source = "# Hello\n\nworld **bold**\n";

function resizableInput() {
const bytes = new TextEncoder().encode(source);
const buffer = new ArrayBuffer(bytes.length, { maxByteLength: bytes.length });
const view = new Uint8Array(buffer);
view.set(bytes);
return { buffer, view };
}

test("html: an option getter shrinks it", () => {
const { buffer, view } = resizableInput();
const options = {
get autolinks() {
buffer.resize(0);
return true;
},
};
expect(Markdown.html(view, options)).toBe(Markdown.html(source, { autolinks: true }));
expect(buffer.byteLength).toBe(0);
});

test("ansi: a theme getter shrinks it", () => {
const { buffer, view } = resizableInput();
const theme = {
get colors() {
buffer.resize(0);
return false;
},
};
expect(Markdown.ansi(view, theme)).toBe(Markdown.ansi(source, { colors: false }));
expect(buffer.byteLength).toBe(0);
});

test("render: a callback shrinks it", () => {
const { buffer, view } = resizableInput();
const result = Markdown.render(view, {
heading: (children: string) => {
buffer.resize(0);
return `<h1>${children}</h1>`;
},
paragraph: (children: string) => `<p>${children}</p>`,
strong: (children: string) => `<b>${children}</b>`,
});
expect(result).toBe("<h1>Hello</h1><p>world <b>bold</b></p>");
expect(buffer.byteLength).toBe(0);
});

test("react: an option getter shrinks it", () => {
const { buffer, view } = resizableInput();
const options = {
reactVersion: 18,
get autolinks() {
buffer.resize(0);
return true;
},
};
expect(Markdown.react(view, undefined, options)).toEqual(
Markdown.react(source, undefined, { reactVersion: 18, autolinks: true }),
);
expect(buffer.byteLength).toBe(0);
});
});
});
Loading
Loading