diff --git a/src/jsc/JSValue.rs b/src/jsc/JSValue.rs index c4b8ab3f5c20..a7bdf686df88 100644 --- a/src/jsc/JSValue.rs +++ b/src/jsc/JSValue.rs @@ -1812,19 +1812,6 @@ impl FromAny for &str { bun_string_jsc::create_utf8_for_js(global, self.as_bytes()) } } -impl FromAny for Box<[bun_core::String]> { - /// The boxed - /// slice is consumed: every element's WTF refcount is dropped and the - /// backing allocation freed via `Box` drop. `bun_core::String` is `Copy` - /// with no `Drop`, so the explicit `deref()` loop is required. - fn into_js_value(self, global: &JSGlobalObject) -> JsResult { - let result = bun_string_jsc::to_js_array(global, &self); - for out in self.iter() { - out.deref(); - } - result - } -} impl FromAny for Option { /// `None` → `undefined`. #[inline] diff --git a/src/runtime/api/JSTranspiler.rs b/src/runtime/api/JSTranspiler.rs index 27b16aa07e26..81a721b86d0d 100644 --- a/src/runtime/api/JSTranspiler.rs +++ b/src/runtime/api/JSTranspiler.rs @@ -654,7 +654,7 @@ impl Config { /// which the job's Js side keeps alive and the pool borrow keeps valid. pub(crate) struct TransformTask { pub input_code: bun_jsc::ThreadSafe, - pub output_code: BunString, + pub output_code: OwnedString, pub transpiler: core::mem::ManuallyDrop>, pub log: bun_ast::Log, pub err: Option, @@ -719,7 +719,7 @@ impl TransformTask { let task = TransformTask { input_code, - output_code: BunString::empty(), + output_code: OwnedString::default(), transpiler: transpiler_copy, macro_map: clone_macro_map(&config.macro_map), tsconfig: config @@ -831,7 +831,6 @@ impl TransformTask { }; if parse_result.empty { - self.output_code = BunString::empty(); return; } @@ -861,9 +860,7 @@ impl TransformTask { buffer_writer = printer.ctx; // `written()` reslices via `written_len`; copy out the printed // bytes, then the local writer is dropped. - self.output_code = BunString::clone_utf8(buffer_writer.written()); - } else { - self.output_code = BunString::empty(); + self.output_code = OwnedString::new(BunString::clone_utf8(buffer_writer.written())); } } @@ -872,8 +869,7 @@ impl TransformTask { promise: &mut JSPromise, global: &JSGlobalObject, ) -> Result<(), bun_jsc::JsTerminated> { - // The job drops this `TransformTask` (running its `Drop`: transpiler - // deref etc.) right after `then` returns. + // The job drops this `TransformTask` right after `then` returns. if self.log.has_any() || self.err.is_some() { let error_value: JsResult = 'brk: { if let Some(err) = &self.err { diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 6046c4ad2fd5..c74a2310e671 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -10,7 +10,7 @@ use core::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use crate::api::bun::process::event_loop_handle_to_ctx; use crate::webcore; use bun_core::Environment; -use bun_core::{String as BunString, ZStr}; +use bun_core::{OwnedString, String as BunString, ZStr}; use bun_event_loop::AnyTaskWithExtraContext::AnyTaskWithExtraContext; use bun_io::KeepAlive; use bun_jsc::AbortSignal; @@ -1197,10 +1197,7 @@ mod _async_tasks { impl FsReturn for ret::Readdir { #[inline] fn fs_to_js(&mut self, global: &JSGlobalObject) -> JsResult { - // `Readdir::to_js` consumes by value (the boxed slices are handed to - // JS). Swap in an empty `Files` payload so `&mut self` stays valid. - let owned = core::mem::replace(self, ret::Readdir::Files(Box::default())); - owned.to_js(global) + self.to_js(global) } } impl FsReturn for StatOrNotFound { @@ -2220,7 +2217,7 @@ mod _async_tasks { } } } else { - let res = match core::mem::replace( + let mut res = match core::mem::replace( &mut this.result_list, ResultListEntryValue::Files(Vec::new()), ) { @@ -4431,7 +4428,7 @@ impl StatOrNotFound { } pub enum StringOrUndefined { - String(BunString), + String(OwnedString), None, } impl StringOrUndefined { @@ -4508,15 +4505,16 @@ pub mod ret { Files, } + /// `to_js` converts in place; `Drop` releases whatever it did not hand over. pub enum Readdir { WithFileTypes(Box<[Dirent]>), Buffers(Box<[Buffer]>), Files(Box<[BunString]>), } impl Readdir { - pub fn to_js(self, global_object: &JSGlobalObject) -> JsResult { + pub fn to_js(&mut self, global_object: &JSGlobalObject) -> JsResult { match self { - Readdir::WithFileTypes(mut items) => { + Readdir::WithFileTypes(items) => { let array = JSValue::create_empty_array(global_object, items.len())?; let mut previous_jsstring: *mut bun_jsc::JSString = core::ptr::null_mut(); for (i, item) in items.iter_mut().enumerate() { @@ -4524,14 +4522,12 @@ pub mod ret { item.to_js_newly_created(global_object, Some(&mut previous_jsstring))?; array.put_index(global_object, i as u32, res)?; } - // items dropped here (auto free) Ok(array) } - Readdir::Buffers(mut items) => { + Readdir::Buffers(items) => { // Node returns `Buffer[]` for `{ encoding: "buffer" }`, not // `Uint8Array[]`. Ownership of every `Buffer`'s bytes - // transfers to JSC via `to_node_buffer`; the boxed slice - // itself is freed when `items` drops. + // transfers to JSC via `to_node_buffer`. let array = JSValue::create_empty_array(global_object, items.len())?; for (i, item) in items.iter_mut().enumerate() { let res = item.to_node_buffer(global_object)?; @@ -4539,12 +4535,19 @@ pub mod ret { } Ok(array) } - Readdir::Files(items) => { - // Converted to a JS array, then every element is - // deref'd and the slice freed (handled by the `FromAny - // for Box<[bun_core::String]>` impl). - JSValue::from_any(global_object, items) - } + // The array takes its own refs; ours go in `Drop`. + Readdir::Files(items) => bun_jsc::bun_string_jsc::to_js_array(global_object, items), + } + } + } + impl Drop for Readdir { + fn drop(&mut self) { + match self { + // Transferred entries are empty by now; `deref` on those is a no-op. + Readdir::WithFileTypes(items) => items.iter().for_each(Dirent::deref), + // `Buffer` frees whatever it still owns itself. + Readdir::Buffers(_) => {} + Readdir::Files(items) => items.iter().for_each(BunString::deref), } } } @@ -5711,8 +5714,8 @@ impl NodeFS { if !RETURN_PATH { return Ok(StringOrUndefined::None); } - return Ok(StringOrUndefined::String(BunString::create_from_os_path( - &path[..], + return Ok(StringOrUndefined::String(OwnedString::new( + BunString::create_from_os_path(&path[..]), ))); } } @@ -5881,8 +5884,8 @@ impl NodeFS { if !RETURN_PATH { return Ok(StringOrUndefined::None); } - Ok(StringOrUndefined::String(BunString::create_from_os_path( - &working_mem[..first_match as usize], + Ok(StringOrUndefined::String(OwnedString::new( + BunString::create_from_os_path(&working_mem[..first_match as usize]), ))) } diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index eb149d320b7a..27ce747bf960 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -4634,26 +4634,16 @@ pub(crate) fn write_file_with_source_destination( let source_type = source_store.data.tag(); if destination_type == store::DataTag::File && source_type == store::DataTag::Bytes { - let write_file_promise = bun_core::heap::into_raw(Box::new(WriteFilePromise { - promise: jsc::JSPromiseStrong::default(), - global_this: ctx, - })); - - // The borrowed views below are +0 on the store ref; - // `WriteFile::create` takes its own ref. #[cfg(windows)] { - let promise = JSPromise::create(ctx); - let promise_value = promise.as_value(ctx); + let promise = WriteFilePromise::init(ctx); + let promise_value = promise.value(); promise_value.ensure_still_alive(); - // SAFETY: write_file_promise was just produced by heap::alloc above; sole owner. - unsafe { (*write_file_promise).promise.set(ctx, promise_value) }; match write_file_mod::WriteFileWindows::create( ctx.bun_vm().event_loop(), destination_blob.borrowed_view(), source_blob.borrowed_view(), - write_file_promise, - WriteFilePromise::run, + promise, options.mkdirp_if_not_exists.unwrap_or(true), ) { Err(write_file_mod::WriteFileWindowsError::WriteFileWindowsDeinitialized) => {} @@ -4670,18 +4660,13 @@ pub(crate) fn write_file_with_source_destination( let file_copier = write_file_mod::WriteFile::create( destination_blob.borrowed_view(), source_blob.borrowed_view(), - write_file_promise, - WriteFilePromise::run, options.mkdirp_if_not_exists.unwrap_or(true), ) .expect("unreachable"); - // Defer promise creation until we're just about to schedule the task. - // SAFETY: write_file_promise was just produced by heap::alloc above; sole owner. - unsafe { (*write_file_promise).promise = jsc::JSPromiseStrong::init(ctx) }; - // SAFETY: same `write_file_promise` as above; still solely owned here. - let promise_value = unsafe { (*write_file_promise).promise.value() }; + let promise = WriteFilePromise::init(ctx); + let promise_value = promise.value(); promise_value.ensure_still_alive(); - write_file_mod::WriteFile::schedule(file_copier, ctx); + write_file_mod::WriteFile::schedule(file_copier, promise, ctx); return Ok(promise_value); } } diff --git a/src/runtime/webcore/blob/write_file.rs b/src/runtime/webcore/blob/write_file.rs index 293d064020a5..31d20b6f42cb 100644 --- a/src/runtime/webcore/blob/write_file.rs +++ b/src/runtime/webcore/blob/write_file.rs @@ -24,30 +24,56 @@ use crate::webcore::body; bun_output::declare_scope!(WriteFile, hidden); -// A tagged result-or-error union. Modeled -// as a plain Rust enum: it only ever travels through the Rust fn-pointer -// callbacks below (`WriteFileOnWriteFileCallback`), never across FFI, so the -// layout is unconstrained. +/// What a finished write settles its promise with. pub enum WriteFileResultType { Result(SizeType), Err(Box), } -pub type WriteFileOnWriteFileCallback = - fn(ctx: *mut c_void, count: WriteFileResultType) -> Result<(), JsTerminated>; +/// The promise `Bun.write()` returned: a `WriteFile` job's JS half, or a `WriteFileWindows` field. +#[derive(bun_jsc::JsAffine)] +pub struct WriteFilePromise { + promise: jsc::JSPromiseStrong, +} + +impl WriteFilePromise { + pub(crate) fn init(global: &JSGlobalObject) -> Self { + Self { + promise: jsc::JSPromiseStrong::init(global), + } + } + + pub(crate) fn value(&self) -> JSValue { + self.promise.value() + } + + pub(crate) fn settle( + mut self, + global: &JSGlobalObject, + result: WriteFileResultType, + ) -> Result<(), JsTerminated> { + match result { + WriteFileResultType::Err(err) => { + let err_js = err.to_error_instance(global); + self.promise.reject_with_async_stack(global, Ok(err_js)) + } + WriteFileResultType::Result(wrote) => self + .promise + .resolve(global, JSValue::js_number_from_uint64(wrote as u64)), + } + } +} /// The completion token a `WriteFile` keeps across its async I/O. pub type WriteFileTask = bun_jsc::Completion; -// SAFETY: the two blobs are native values holding store refs (atomic counts); -// io-loop registration state and an opaque completion ctx that only the -// JS-thread completion dereferences — nothing used off-thread is thread-affine. +// SAFETY: the two blobs hold store refs (atomic counts) and the rest is io-loop +// registration state; the promise lives on the job's JS side, never here. unsafe impl Send for WriteFile {} impl bun_jsc::JobContext for WriteFile { type OffThread = Self; - /// The completion is delivered through `on_complete_callback(ctx, ..)`. - type Js = (); + type Js = WriteFilePromise; fn run( this: &mut Self, _vm: &bun_jsc::vm_handle::Borrow, @@ -57,16 +83,20 @@ impl bun_jsc::JobContext for WriteFile { this.run(done); None } - fn then(this: Self, _: (), cx: &bun_jsc::JsThread<'_>) -> jsc::JsResult<()> { - Ok(WriteFile::then(this, cx.global())?) + fn then( + this: Self, + promise: WriteFilePromise, + cx: &bun_jsc::JsThread<'_>, + ) -> jsc::JsResult<()> { + Ok(WriteFile::then(this, promise, cx.global())?) } } +#[cfg(not(windows))] impl WriteFile { - /// JS thread: hand a prepared `WriteFile` to the work pool (the job is - /// its one heap allocation). - pub fn schedule(this: WriteFile, global: &JSGlobalObject) { - bun_jsc::Job::::schedule(&global.js_thread(), this, ()); + /// JS thread: hand the write and the promise it settles to the work pool. + pub(crate) fn schedule(this: WriteFile, promise: WriteFilePromise, global: &JSGlobalObject) { + bun_jsc::Job::::schedule(&global.js_thread(), this, promise); } } @@ -85,8 +115,6 @@ pub struct WriteFile { pub(crate) io_request: io::Request, pub(crate) state: AtomicU8, // ClosingState - pub(crate) on_complete_ctx: *mut c_void, - pub(crate) on_complete_callback: WriteFileOnWriteFileCallback, pub(crate) total_written: usize, #[cfg(not(windows))] @@ -230,11 +258,9 @@ impl WriteFile { } #[cfg(not(windows))] - pub(crate) fn create_with_ctx( + pub(crate) fn create( file_blob: Blob, bytes_blob: Blob, - on_write_file_context: *mut c_void, - on_complete_callback: WriteFileOnWriteFileCallback, mkdirp_if_not_exists: bool, ) -> Result { let write_file = WriteFile { @@ -251,39 +277,17 @@ impl WriteFile { io_poll: io::Poll::default(), io_request: io::Request::new(Self::on_request_writable), state: AtomicU8::new(ClosingState::Running as u8), - on_complete_ctx: on_write_file_context, - on_complete_callback, total_written: 0, could_block: false, close_after_io: false, mkdirp_if_not_exists, }; - // No explicit store ref bump: the caller passes a `+1` Blob (via + // No explicit store ref bump: the caller passes `+1` Blobs (via // `borrowed_view()`'s `StoreRef::clone`) and dropping the `WriteFile` - // in `then` runs `StoreRef::drop`, so the ref/deref pair is RAII. + // runs `StoreRef::drop`, so the ref/deref pair is RAII. Ok(write_file) } - #[cfg(not(windows))] - pub(crate) fn create( - file_blob: Blob, - bytes_blob: Blob, - context: *mut C, - callback: WriteFileOnWriteFileCallback, - mkdirp_if_not_exists: bool, - ) -> Result { - // The caller supplies a - // `*mut c_void`-typed callback directly (see `WriteFilePromise::run`), - // so this is just a `.cast()` on `context`. - WriteFile::create_with_ctx( - file_blob, - bytes_blob, - context.cast::(), - callback, - mkdirp_if_not_exists, - ) - } - // reshaped for borrowck — take (off, len) here and re-derive the slice // internally so callers don't hold a borrow of self across the &mut self call. #[cfg(not(windows))] @@ -327,30 +331,18 @@ impl WriteFile { true } - pub(crate) fn then(mut this: WriteFile, _global: &JSGlobalObject) -> Result<(), JsTerminated> { - let cb = this.on_complete_callback; - let cb_ctx = this.on_complete_ctx; - let system_error = this.system_error.take(); - let total_written = this.total_written; - // Cleanup is RAII: dropping the `Box` runs `WriteFile`'s field-drop - // glue, which drops `bytes_blob.store`/`file_blob.store: Option< - // StoreRef>` → `Store::deref()` — exactly one deref each. - // (An earlier explicit `detach()` here was a no-op; the - // bun-write-leak.test.ts failure was the ASAN debug build's ~320 MB - // baseline RSS exceeding the fixture's 256 MB absolute threshold, - // not an unbalanced ref.) + pub(crate) fn then( + mut this: WriteFile, + promise: WriteFilePromise, + global: &JSGlobalObject, + ) -> Result<(), JsTerminated> { + let result = match this.system_error.take() { + Some(err) => WriteFileResultType::Err(Box::new(err)), + None => WriteFileResultType::Result(this.total_written as SizeType), + }; + // Releases both blobs' store refs; nothing to detach by hand. drop(this); - - if let Some(err) = system_error { - cb(cb_ctx, WriteFileResultType::Err(Box::new(err)))?; - return Ok(()); - } - - cb( - cb_ctx, - WriteFileResultType::Result(total_written as SizeType), - )?; - Ok(()) + promise.settle(global, result) } pub(crate) fn run(&mut self, task: WriteFileTask) { @@ -572,8 +564,8 @@ mod windows_impl { pub(crate) io_request: uv::fs_t, pub(crate) file_blob: Blob, pub(crate) bytes_blob: Blob, - pub(crate) on_complete_callback: WriteFileOnWriteFileCallback, - pub(crate) on_complete_ctx: *mut c_void, + /// `Some` until `run_from_js_thread` settles it. + pub(crate) promise: Option, pub(crate) mkdirp_if_not_exists: bool, pub(crate) uv_bufs: [uv::uv_buf_t; 1], @@ -611,12 +603,12 @@ mod windows_impl { } impl WriteFileWindows { - pub(crate) fn create_with_ctx( + /// `Err(WriteFileWindowsDeinitialized)`: failed synchronously, `promise` already rejected. + pub(crate) fn create( + event_loop: *mut EventLoop, file_blob: Blob, bytes_blob: Blob, - event_loop: *mut EventLoop, - on_write_file_context: *mut c_void, - on_complete_callback: WriteFileOnWriteFileCallback, + promise: WriteFilePromise, mkdirp_if_not_exists: bool, ) -> Result<*mut WriteFileWindows, WriteFileWindowsError> { let mkdirp = mkdirp_if_not_exists @@ -632,8 +624,7 @@ mod windows_impl { let write_file = Self::new(WriteFileWindows { file_blob, bytes_blob, - on_complete_ctx: on_write_file_context, - on_complete_callback, + promise: Some(promise), mkdirp_if_not_exists: mkdirp, io_request: bun_core::ffi::zeroed::(), uv_bufs: [uv::uv_buf_t { @@ -814,7 +805,7 @@ mod windows_impl { let this: *mut WriteFileWindows = unsafe { WriteFileWindows::from_uv_fs(req) }; debug_assert!(core::ptr::eq( this, - // SAFETY: req == &(*this).io_request; data was set to `this` in create_with_ctx/open. + // SAFETY: req == &(*this).io_request; data was set to `this` in create/open. unsafe { (*req).data }.cast::() )); // SAFETY: `this` is live (libuv invokes us with the req we registered). @@ -1049,28 +1040,29 @@ mod windows_impl { /// `this` must point to a live `WriteFileWindows` allocated via [`Self::new`]. /// On return, `*this` has been freed and must not be accessed again. pub(crate) unsafe fn run_from_js_thread(this: *mut Self) -> WriteFileWindowsError { - // SAFETY: caller contract — `this` is live; copy out everything we + // SAFETY: caller contract — `this` is live; take out everything we // need before `deinit` frees the allocation. - let (cb, cb_ctx) = unsafe { ((*this).on_complete_callback, (*this).on_complete_ctx) }; + let (promise, global, result) = unsafe { + let result = match (*this).to_system_error() { + Some(err) => WriteFileResultType::Err(Box::new(err)), + None => WriteFileResultType::Result((*this).total_written as SizeType), + }; + ( + (*this) + .promise + .take() + .expect("a WriteFileWindows finishes once"), + (*(*this).event_loop).global_ref(), + result, + ) + }; + // SAFETY: caller contract — `this` is live; consumed here. + unsafe { Self::deinit(this) }; - // SAFETY: caller contract — `this` is live. - if let Some(err) = unsafe { (*this).to_system_error() } { - // SAFETY: caller contract — `this` is live; consumed here. - unsafe { Self::deinit(this) }; - if let Err(e) = cb(cb_ctx, WriteFileResultType::Err(Box::new(err))) { - return e.into(); - } - } else { - // SAFETY: caller contract — `this` is live. - let wrote = unsafe { (*this).total_written }; - // SAFETY: caller contract — `this` is live; consumed here. - unsafe { Self::deinit(this) }; - if let Err(e) = cb(cb_ctx, WriteFileResultType::Result(wrote as SizeType)) { - return e.into(); - } + match promise.settle(global, result) { + Ok(()) => WriteFileWindowsError::WriteFileWindowsDeinitialized, + Err(terminated) => terminated.into(), } - - WriteFileWindowsError::WriteFileWindowsDeinitialized } /// # Safety @@ -1200,7 +1192,7 @@ mod windows_impl { aio::Closer::close(Fd::from_uv(fd), (*this).io_request.loop_); } // The store derefs happen via `StoreRef::drop` when the Box is - // reclaimed below (paired with the RAII note in `create_with_ctx`). + // reclaimed below (paired with the RAII note in `create`). (*this).poll_ref.disable(); // (*this).io_request is a valid uv_fs_t embedded in this struct; uv_fs_req_cleanup // is safe on a zeroed or previously-used req. @@ -1209,74 +1201,6 @@ mod windows_impl { drop(bun_core::heap::take(this)); } } - - pub(crate) fn create( - event_loop: *mut EventLoop, - file_blob: Blob, - bytes_blob: Blob, - context: *mut C, - callback: WriteFileOnWriteFileCallback, - mkdirp_if_not_exists: bool, - ) -> Result<*mut WriteFileWindows, WriteFileWindowsError> { - // see `WriteFile::create` — caller supplies an erased - // `*mut c_void` callback directly; `context` is just `.cast()`ed. - WriteFileWindows::create_with_ctx( - file_blob, - bytes_blob, - event_loop, - context.cast::(), - callback, - mkdirp_if_not_exists, - ) - } - } -} - -// ────────────────────────────────────────────────────────────────────────── - -pub struct WriteFilePromise { - pub(crate) promise: jsc::JSPromiseStrong, - pub global_this: *const JSGlobalObject, -} - -impl WriteFilePromise { - pub(crate) fn run( - handler: *mut c_void, - count: WriteFileResultType, - ) -> Result<(), JsTerminated> { - let handler = handler.cast::(); - // SAFETY: handler is the Box-allocated WriteFilePromise created in - // Blob.rs (`heap::into_raw(Box::new(WriteFilePromise { .. }))`); consumed here. - // `swap()` releases the Strong's handle slot and yields a GC-owned `*mut JSPromise`, - // which stays valid past `drop(heap::take(handler))`. - let (promise, global_this): (*mut JSPromise, &JSGlobalObject) = unsafe { - let h = &mut *handler; - let promise = std::ptr::from_mut::(h.promise.swap()); - let global_this = &*h.global_this; - drop(bun_core::heap::take(handler)); - (promise, global_this) - }; - // SAFETY: GC-owned cell (kept alive below); scoped shared access. - let value = unsafe { (*promise).to_js() }; - value.ensure_still_alive(); - match count { - WriteFileResultType::Err(err) => { - // SAFETY: GC-owned cell; the error build's shared borrow ends before the - // scoped exclusive `reject` borrow. - unsafe { - let err_js = err.to_error_instance_with_async_stack(global_this, &*promise); - (*promise).reject(global_this, Ok(err_js))?; - } - } - WriteFileResultType::Result(wrote) => { - // SAFETY: GC-owned cell; exclusive borrow scoped to the call. - unsafe { - (*promise) - .resolve(global_this, JSValue::js_number_from_uint64(wrote as u64))?; - } - } - } - Ok(()) } } diff --git a/test/js/web/workers/worker-refused-completion.test.ts b/test/js/web/workers/worker-refused-completion.test.ts index 20936d3485ed..10522dbda60f 100644 --- a/test/js/web/workers/worker-refused-completion.test.ts +++ b/test/js/web/workers/worker-refused-completion.test.ts @@ -7,14 +7,17 @@ // stderr. A row passes only if the named refusal happened (the work really was // on another thread and its release path ran) and the process exited cleanly; // on the ASAN build the release path is also checked for use-after-free and -// leaks. Builds with debug assertions only (debug, ASAN): the gate does not -// exist in release builds. +// leaks (LSan is turned on below, as CI's ASAN lane does for every test). +// Builds with debug assertions only (debug, ASAN): the gate does not exist in +// release builds. import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, isASAN, isDebug, isWindows } from "harness"; +import { bunEnv, bunExe, isASAN, isDebug, isWindows, tempDir } from "harness"; +import { join } from "node:path"; type Row = { name: string; - // Runs in the worker before it exits; starts exactly one piece of off-thread work. + // Runs in the worker before it exits (cwd: a scratch directory holding + // SCRATCH_FILES); starts exactly one piece of off-thread work. worker: string; // Substring of the refusal the runtime must log for it. refused: string; @@ -36,10 +39,13 @@ const ROWS: Row[] = [ worker: `require("node:fs").realpath(process.execPath, () => {});`, refused: "args::Realpath", }, + // On Windows these three are libuv requests on the worker's own loop + // (ReadFileUV, WriteFileWindows), not pool jobs: nothing gets refused. { name: "Bun.file().text()", worker: `Bun.file(process.execPath).slice(0, 65536).text();`, refused: "blob::read_file::ReadFile", + skip: isWindows, }, { // Same read job, different completion: the image's read chain is handed @@ -47,6 +53,32 @@ const ROWS: Row[] = [ name: "Bun.Image(Bun.file()).metadata()", worker: `new Bun.Image(Bun.file(process.execPath).slice(0, 65536)).metadata().catch(() => {});`, refused: "blob::read_file::ReadFile", + skip: isWindows, + }, + { + // A Blob source: a string or buffer this small is written synchronously instead. + name: "Bun.write()", + worker: `Bun.write("written-by-the-pool", new Blob(["refused"]));`, + refused: "blob::write_file::WriteFile", + skip: isWindows, + }, + // The result the pool produced (names / Dirents / the created path / the + // transpiled code) is what must be released along with the refused job. + { name: "fs.promises.readdir", worker: `require("node:fs").promises.readdir(".");`, refused: "args::Readdir" }, + { + name: "fs.promises.readdir withFileTypes", + worker: `require("node:fs").promises.readdir(".", { withFileTypes: true });`, + refused: "args::Readdir", + }, + { + name: "fs.promises.mkdir recursive", + worker: `require("node:fs").promises.mkdir("made/by/the/pool", { recursive: true });`, + refused: "args::Mkdir", + }, + { + name: "Bun.Transpiler transform()", + worker: `new Bun.Transpiler({ loader: "ts" }).transform("export const refused: number = 1;");`, + refused: "TransformTask", }, { name: "crypto.pbkdf2", @@ -107,11 +139,19 @@ const ROWS: Row[] = [ }, ]; +// The host's cwd: what the readdir rows list, where the write and mkdir rows +// put what the pool produces; thrown away with the row. +const SCRATCH_FILES = { "a.txt": "", "b.txt": "", "c.txt": "", "d.txt": "" }; + // The host: one worker, armed gate. The worker starts its work, reports // "armed" and exits by itself two turns later; the parent never posts anything // the worker waits for (with the gate armed, a parent→worker post is itself a // cross-thread completion that waits for the worker's close). Rows with a // parent side post in response to "armed": that post is what gets refused. +// +// The row's work starts from a microtask: test/leaksan.supp suppresses every +// allocation made under the module evaluation (leak:Bun::evaluateCommonJSModuleOnce), +// which would also hide what a row allocates while starting its work. function host(row: Row) { return ` const { Worker, MessageChannel } = require("node:worker_threads"); @@ -119,7 +159,7 @@ function host(row: Row) { const w = new Worker(\` const { parentPort, workerData } = require("node:worker_threads"); parentPort.on("message", () => {}); - ${row.worker.replace(/`/g, "\\`").replace(/\$\{/g, "\\${")} + queueMicrotask(() => { ${row.worker.replace(/`/g, "\\`").replace(/\$\{/g, "\\${")} }); parentPort.postMessage("armed"); setImmediate(() => setImmediate(() => process.exit(0))); \`, { eval: true, workerData: { port: port2 }, transferList: [port2] }); @@ -129,26 +169,47 @@ function host(row: Row) { `; } +// What a refused job failed to release shows up as an LSan report, and a +// non-zero exit, on the ASAN build (the same options CI's ASAN lane sets). +// LSan cannot see into the JS heap, so the host's own VM has to be torn down +// at exit too, or everything its wrappers still own would be reported. +const LEAK_CHECK_ENV = isASAN + ? { + BUN_DESTRUCT_VM_ON_EXIT: "1", + ASAN_OPTIONS: [bunEnv.ASAN_OPTIONS, "detect_leaks=1"].filter(Boolean).join(":"), + LSAN_OPTIONS: `print_suppressions=0:suppressions=${join(import.meta.dirname, "../../../leaksan.supp")}`, + } + : {}; + describe.skipIf(!isDebug && !isASAN)( "a completion for a worker that is gone is refused and released by its producer", () => { for (const row of ROWS) { - test.concurrent.skipIf(!!row.skip)(row.name, async () => { - await using proc = Bun.spawn({ - cmd: [bunExe(), "-e", host(row)], - env: { ...bunEnv, ...row.env, BUN_DEBUG_TEST_WORKER_REFUSAL_GATE: "1" }, - stdout: "pipe", - stderr: "pipe", - }); - const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - const refusals = stderr.split("\n").filter(l => l.startsWith("[vm_handle] refused ")); - expect({ - exitCode, - refused: refusals.some(l => l.includes(row.refused)), - // On a failure, everything the host printed. - detail: exitCode === 0 && refusals.some(l => l.includes(row.refused)) ? "" : stdout + stderr, - }).toEqual({ exitCode: 0, refused: true, detail: "" }); - }); + test.concurrent.skipIf(!!row.skip)( + row.name, + async () => { + using scratch = tempDir("worker-refused-completion", SCRATCH_FILES); + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", host(row)], + cwd: String(scratch), + env: { ...bunEnv, ...LEAK_CHECK_ENV, ...row.env, BUN_DEBUG_TEST_WORKER_REFUSAL_GATE: "1" }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const refusals = stderr.split("\n").filter(l => l.startsWith("[vm_handle] refused ")); + expect({ + exitCode, + refused: refusals.some(l => l.includes(row.refused)), + // On a failure, everything the host printed. + detail: exitCode === 0 && refusals.some(l => l.includes(row.refused)) ? "" : stdout + stderr, + }).toEqual({ exitCode: 0, refused: true, detail: "" }); + }, + // Only debug and ASAN builds get here (see the describe), and on those + // one worker lifecycle is a few seconds of CPU; bun test starts twenty + // of these hosts at once, so the slowest rows pass 5s on a busy box. + 15_000, + ); } }, );