From ca1affdf74137434c37253a9b44311326b69bafe Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 01:05:10 +0000 Subject: [PATCH 1/4] Let pool jobs that own their memory run without a VM borrow, so terminate() does not wait for them Job::run_on_pool took a VM borrow around every JobContext's run(), so a worker's teardown waited for whatever the body was doing. CopyFile does the whole copy inside run() and opens its source blocking, so a Bun.write(file, file) from a FIFO, tty or idle pipe kept the borrow until the other side acted and worker.terminate() never settled. getaddrinfo (dns.lookup) and Bun.secrets had the same shape. JobContext now declares what run() holds on the VM: `type Vm = Borrow` for bodies that reach VM-owned memory (the carrier borrows as before), `type Vm = Unborrowed` for bodies that own everything they touch (the carrier only skips a job whose VM is already closed; a VM torn down under a running body refuses its completion, which the job releases on the pool thread as it already did for a late post). CopyFile, ReadFile, WriteFile, the libc dns lookup, Bun.secrets, Bun.password, Glob.scan and Bun.Archive run unborrowed; the jobs that read caller buffers or write into JS-allocated ones keep the borrow. Bun.secrets' C++ runTask never used the global it was handed. For the file jobs to own their memory the store has to own its path: Bun.file(bytes) and Bun.write(bytes, ...) kept a pinned PathLike::Buffer into the caller's ArrayBuffer in the Store (so the path followed later writes to the buffer, the protect taken for it was never released, and dropping the store off the JS thread would unpin a dead heap). Store::init_file / init_s3 copy a byte path into owned bytes on the JS thread, which is what the Bun.file(bytes) documentation says happens. --- src/jsc/JSSecrets.rs | 14 ++-- src/jsc/VmHandle.rs | 14 +++- src/jsc/bindings/JSSecrets.cpp | 5 +- src/jsc/job.rs | 81 +++++++++++++++---- src/jsc/lib.rs | 2 +- src/runtime/api/Archive.rs | 7 +- src/runtime/api/BunObject.rs | 2 + src/runtime/api/JSTranspiler.rs | 1 + src/runtime/api/glob.rs | 3 +- src/runtime/crypto/PBKDF2.rs | 2 + src/runtime/crypto/PasswordObject.rs | 3 +- src/runtime/dns_jsc/dns.rs | 5 +- src/runtime/image/Image.rs | 1 + src/runtime/node/node_crypto_binding.rs | 3 + src/runtime/node/node_fs.rs | 6 ++ src/runtime/webcore/Blob.rs | 1 - src/runtime/webcore/CompressionStreamCoder.rs | 2 + src/runtime/webcore/blob/Store.rs | 26 +++++- src/runtime/webcore/blob/copy_file.rs | 6 +- src/runtime/webcore/blob/read_file.rs | 5 +- src/runtime/webcore/blob/write_file.rs | 4 +- test/js/bun/util/bun-file.test.ts | 26 ++++++ .../workers/worker-refused-completion.test.ts | 8 ++ .../workers/worker-terminate-lifetime.test.ts | 45 ++++++++++- 24 files changed, 232 insertions(+), 40 deletions(-) diff --git a/src/jsc/JSSecrets.rs b/src/jsc/JSSecrets.rs index 4c58fc528526..406fd2bce10b 100644 --- a/src/jsc/JSSecrets.rs +++ b/src/jsc/JSSecrets.rs @@ -9,7 +9,7 @@ bun_opaque::opaque_ffi! { pub struct SecretsJobOptions; } // to the cell. `deinit` consumes/frees the C++ allocation and so stays // `unsafe fn` (double-free precondition). unsafe extern "C" { - safe fn Bun__SecretsJobOptions__runTask(ctx: &mut SecretsJobOptions, global: &JSGlobalObject); + safe fn Bun__SecretsJobOptions__runTask(ctx: &mut SecretsJobOptions); safe fn Bun__SecretsJobOptions__runFromJS( ctx: &mut SecretsJobOptions, global: &JSGlobalObject, @@ -32,21 +32,21 @@ impl Drop for SecretsOptions { /// `Bun.secrets.{get,set,delete}` off the JS thread. pub(crate) struct SecretsJob { options: SecretsOptions, - global: crate::JsPtr, } impl crate::JobContext for SecretsJob { type OffThread = Self; type Js = Strong; + /// The credential store (Keychain, libsecret's D-Bus service, ...) may sit + /// on a user prompt indefinitely; the job owns everything it hands it. + type Vm = crate::Unborrowed; fn run( this: &mut Self, - vm: &crate::vm_handle::Borrow, + _: &crate::Unborrowed, done: crate::Completion, ) -> Option> { - // SAFETY: the creating global, alive under the borrow; C++ only threads it through. - let global = unsafe { this.global.under_borrow(vm) }; - Bun__SecretsJobOptions__runTask(SecretsJobOptions::opaque_mut(this.options.0), global); + Bun__SecretsJobOptions__runTask(SecretsJobOptions::opaque_mut(this.options.0)); Some(done) } @@ -79,8 +79,6 @@ extern "C" fn Bun__Secrets__scheduleJob( &cx, SecretsJob { options: SecretsOptions(options), - // SAFETY: the creating global outlives every borrow of its VM. - global: unsafe { crate::JsPtr::new(core::ptr::NonNull::from(global)) }, }, Strong::create(promise, global), ); diff --git a/src/jsc/VmHandle.rs b/src/jsc/VmHandle.rs index 1a536254a518..67586ef49e4e 100644 --- a/src/jsc/VmHandle.rs +++ b/src/jsc/VmHandle.rs @@ -247,7 +247,8 @@ impl VmHandle { /// This job is about to use VM-owned memory off-thread; `None` if the VM /// is closed (touch nothing). Hold the result until done. Jobs that could - /// block indefinitely on an external party must own their memory instead. + /// block indefinitely on an external party must own their memory instead + /// and only ask [`is_closed`](Self::is_closed) whether to bother starting. pub fn borrow(&self) -> Option { let a = self.enter()?; // SAFETY: lifetime extension is sound because `Borrow` also holds a @@ -267,6 +268,14 @@ impl VmHandle { (self.0.hot.state.load(Ordering::SeqCst) == State::Open as u8).then_some(b) } + /// Whether `close()` has run, i.e. whatever this work would post is going + /// to be refused. Any thread; a snapshot, so only for deciding not to start + /// work that owns its memory — work that uses the VM's takes a + /// [`borrow`](Self::borrow). + pub fn is_closed(&self) -> bool { + self.0.hot.state.load(Ordering::SeqCst) == State::Closed as u8 + } + // ── embedded work ───────────────────────────────────────────────────── // // Pool work whose storage is a field of a JS-owned object (a transpile @@ -801,6 +810,9 @@ impl LoopHandle { pub fn borrow_if_running(&self) -> Option { self.vm.borrow_if_running() } + pub fn is_closed(&self) -> bool { + self.vm.is_closed() + } pub fn accepting_work(&self) -> bool { self.vm.accepting_work() } diff --git a/src/jsc/bindings/JSSecrets.cpp b/src/jsc/bindings/JSSecrets.cpp index d28dcde973f9..c8d571d83896 100644 --- a/src/jsc/bindings/JSSecrets.cpp +++ b/src/jsc/bindings/JSSecrets.cpp @@ -242,8 +242,9 @@ struct SecretsJobOptions { // C interface implementation for the native binding extern "C" { -// Runs on the threadpool - does the actual platform API work -void Bun__SecretsJobOptions__runTask(SecretsJobOptions* opts, JSGlobalObject* global) +// Runs on the threadpool - does the actual platform API work. The job's VM may +// be torn down while this blocks (a keychain prompt), so it gets nothing of it. +void Bun__SecretsJobOptions__runTask(SecretsJobOptions* opts) { // Already have CString fields, pass them directly to platform APIs switch (opts->op) { diff --git a/src/jsc/job.rs b/src/jsc/job.rs index d71a8b480ee0..6d385ac678d1 100644 --- a/src/jsc/job.rs +++ b/src/jsc/job.rs @@ -4,11 +4,14 @@ //! [`WorkPool`], and completes on the JS thread again — unless its VM went away //! meanwhile. Which thread may touch which part of it is in the types: //! -//! * [`JobContext::OffThread`] is what the pool body sees. It is `Send`, and it -//! runs under a VM [`Borrow`] the carrier takes for it, so the VM's teardown -//! waits for a body that is mid-flight and a body never starts against a VM -//! that is already closed. JS-backed memory it needs is reachable only through -//! [`JsPtr`], i.e. only while that borrow (or a [`JsThread`]) is in hand. +//! * [`JobContext::OffThread`] is what the pool body sees. It is `Send`, and a +//! body never starts against a VM that is already closed. Whether the VM's +//! teardown also *waits* for a body that is mid-flight is the impl's +//! [`JobContext::Vm`]: a body that reads VM-owned memory runs under a VM +//! [`Borrow`] the carrier takes for it (JS-backed memory is reachable only +//! through [`JsPtr`], i.e. only while that borrow, or a [`JsThread`], is in +//! hand); a body that owns everything it touches runs [`Unborrowed`], and a +//! VM torn down underneath it simply refuses its completion. //! * [`JobContext::Js`] is the completion's JS-thread state (promise, callback, //! wrapper refs, pins, protected buffers). It is [`JsAffine`] and lives in a //! [`JsSide`], which opens only with a [`JsThread`] token and is never dropped @@ -234,14 +237,29 @@ pub trait JobContext: Sized + 'static { type OffThread: Send; type Js: JsAffine; - /// Pool thread, under a VM borrow the carrier holds for the whole call. - /// Return `done` to complete now; keep it (e.g. across async I/O that - /// finishes on another thread) and call [`Completion::finish`] later to - /// complete then. Work that outlives this call runs under no borrow and - /// must touch only `off`. + /// What [`run`](Self::run) holds on the VM while it executes: + /// + /// * [`Borrow`] — `OffThread` reaches memory the VM owns (a [`JsPtr`], the + /// bytes of a pinned buffer), so the body runs under a VM borrow and the + /// VM's teardown waits for it. Only for bodies that cannot block on an + /// external party: `terminate()` of the worker waits exactly as long. + /// * [`Unborrowed`] — `OffThread` owns everything the body touches, so + /// teardown does not wait for it: a body still running when its VM goes + /// away (a copy blocked on a FIFO, `getaddrinfo`) finishes on its own + /// time and its completion is refused, i.e. for such a job the release on + /// the pool thread ([`Postable::release_refused`]) is the normal end of + /// in-flight work at teardown, not a rare race. + /// + /// [`Postable::release_refused`]: crate::Postable::release_refused + type Vm: VmHold; + + /// Pool thread, with [`Self::Vm`] held for the whole call. Return `done` to + /// complete now; keep it (e.g. across async I/O that finishes on another + /// thread) and call [`Completion::finish`] later to complete then. Work + /// that outlives this call runs under no borrow and must touch only `off`. fn run( off: &mut Self::OffThread, - vm: &Borrow, + vm: &Self::Vm, done: Completion, ) -> Option>; @@ -250,6 +268,39 @@ pub trait JobContext: Sized + 'static { fn then(off: Self::OffThread, js: Self::Js, cx: &JsThread<'_>) -> JsResult<()>; } +mod sealed { + pub trait Sealed {} + impl Sealed for super::Borrow {} + impl Sealed for super::Unborrowed {} +} + +/// What the carrier holds on the VM while [`JobContext::run`] executes: see +/// [`JobContext::Vm`]. Implemented by [`Borrow`] and [`Unborrowed`] only. +pub trait VmHold: sealed::Sealed + Sized { + /// Pool thread, before the body: `None` if the VM is already closed, in + /// which case the body does not run. + fn acquire(handle: &LoopHandle) -> Option; +} + +impl VmHold for Borrow { + #[inline] + fn acquire(handle: &LoopHandle) -> Option { + handle.borrow() + } +} + +/// The [`JobContext::Vm`] of a job whose off-thread part owns everything its +/// body touches: nothing is held, so the VM's teardown does not wait for the +/// body. +pub struct Unborrowed(()); + +impl VmHold for Unborrowed { + #[inline] + fn acquire(handle: &LoopHandle) -> Option { + (!handle.is_closed()).then_some(Unborrowed(())) + } +} + /// The type-erased head of every [`Job`]: dispatch entries (one task tag /// serves every `C`) and the VM's live-job links. #[repr(C)] @@ -383,11 +434,12 @@ impl Job { // SAFETY: live job, exclusively the pool's for this callback. let handle = unsafe { (*this).loop_handle.clone() }; let done = Completion(NonNull::new(this).expect("job")); - let Some(vm) = handle.borrow() else { + let Some(vm) = C::Vm::acquire(&handle) else { // VM already gone: nothing ran; `finish` releases. return done.finish(); }; - // SAFETY: as above; the borrow keeps the VM (and any JsPtr target) alive. + // SAFETY: as above; a `Borrow` keeps the VM (and any JsPtr target) + // alive, and an `Unborrowed` body touches only `off`. if let Some(done) = C::run(unsafe { &mut (*this).off }, &vm, done) { drop(vm); done.finish(); @@ -541,7 +593,8 @@ pub enum Never {} impl JobContext for Never { type OffThread = (); type Js = (); - fn run(_: &mut (), _: &Borrow, done: Completion) -> Option> { + type Vm = Unborrowed; + fn run(_: &mut (), _: &Unborrowed, done: Completion) -> Option> { Some(done) } fn then(_: (), _: (), _: &JsThread<'_>) -> JsResult<()> { diff --git a/src/jsc/lib.rs b/src/jsc/lib.rs index 377b657f5ab7..f457b7aae2b2 100644 --- a/src/jsc/lib.rs +++ b/src/jsc/lib.rs @@ -1388,7 +1388,7 @@ pub use self::event_loop::{ JsTerminatedResult, ManagedTask, MiniEventLoop, PosixSignalHandle, PosixSignalTask, Task, WorkPool, WorkPoolTask, }; -pub use self::job::{Completion, Job, JobContext, JsPtr, JsSide, JsThread, Protected}; +pub use self::job::{Completion, Job, JobContext, JsPtr, JsSide, JsThread, Protected, Unborrowed}; #[cfg(unix)] pub type PlatformEventLoop = bun_uws::Loop; #[cfg(not(unix))] diff --git a/src/runtime/api/Archive.rs b/src/runtime/api/Archive.rs index 33922cf458e1..5c8575eddae3 100644 --- a/src/runtime/api/Archive.rs +++ b/src/runtime/api/Archive.rs @@ -674,7 +674,9 @@ impl PromiseResult { /// One `Bun.Archive` operation's pool-side work: `run` on the thread pool /// stores its result on `self`; `run_from_js` turns it into the promise's -/// value. It is the off-thread part of an `AsyncTask` job. +/// value. It is the off-thread part of an `AsyncTask` job, which runs +/// [`Unborrowed`](bun_jsc::Unborrowed): a context owns everything `run` +/// touches (a store ref, copied arguments), never JS memory. pub trait TaskContext: Send + 'static { /// Runs on thread pool. Stores its result on `self`. fn run(&mut self); @@ -687,9 +689,10 @@ pub struct AsyncTask(core::marker::PhantomData); impl bun_jsc::JobContext for AsyncTask { type OffThread = C; type Js = JSPromiseStrong; + type Vm = bun_jsc::Unborrowed; fn run( ctx: &mut C, - _vm: &bun_jsc::vm_handle::Borrow, + _: &bun_jsc::Unborrowed, done: bun_jsc::Completion, ) -> Option> { ctx.run(); diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index fdedb2be588f..3134ef139fc5 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -2855,6 +2855,8 @@ pub mod JSZstd { impl jsc::JobContext for ZstdJob { type OffThread = Self; type Js = jsc::JSPromiseStrong; + /// `buffer` may be the caller's JS buffer, read below. + type Vm = jsc::vm_handle::Borrow; fn run( this: &mut Self, diff --git a/src/runtime/api/JSTranspiler.rs b/src/runtime/api/JSTranspiler.rs index 27b16aa07e26..8d8c28fcbe71 100644 --- a/src/runtime/api/JSTranspiler.rs +++ b/src/runtime/api/JSTranspiler.rs @@ -677,6 +677,7 @@ pub(crate) struct TransformJs { impl jsc::JobContext for TransformTask { type OffThread = Self; type Js = TransformJs; + type Vm = jsc::vm_handle::Borrow; fn run( this: &mut Self, vm: &jsc::vm_handle::Borrow, diff --git a/src/runtime/api/glob.rs b/src/runtime/api/glob.rs index a44ab7a77e96..cf2c6247a704 100644 --- a/src/runtime/api/glob.rs +++ b/src/runtime/api/glob.rs @@ -246,10 +246,11 @@ impl WalkTaskErr { impl JobContext for WalkTask { type OffThread = Self; type Js = WalkJs; + type Vm = bun_jsc::Unborrowed; fn run( this: &mut Self, - _vm: &bun_jsc::vm_handle::Borrow, + _: &bun_jsc::Unborrowed, done: bun_jsc::Completion, ) -> Option> { let result = match this.walker.walk() { diff --git a/src/runtime/crypto/PBKDF2.rs b/src/runtime/crypto/PBKDF2.rs index e70ffa4f1a70..84171ad83b36 100644 --- a/src/runtime/crypto/PBKDF2.rs +++ b/src/runtime/crypto/PBKDF2.rs @@ -269,6 +269,8 @@ pub(crate) struct Pbkdf2Job { impl JobContext for Pbkdf2Job { type OffThread = Self; type Js = JSPromiseStrong; + /// The password and salt may be the caller's JS buffers, read by `run`. + type Vm = bun_jsc::vm_handle::Borrow; fn run( this: &mut Self, diff --git a/src/runtime/crypto/PasswordObject.rs b/src/runtime/crypto/PasswordObject.rs index 2b2bb24c255b..1cb714be0e17 100644 --- a/src/runtime/crypto/PasswordObject.rs +++ b/src/runtime/crypto/PasswordObject.rs @@ -567,9 +567,10 @@ impl Drop for PasswordJob { impl bun_jsc::JobContext for PasswordJob { type OffThread = Self; type Js = JSPromiseStrong; + type Vm = bun_jsc::Unborrowed; fn run( this: &mut Self, - _vm: &bun_jsc::vm_handle::Borrow, + _: &bun_jsc::Unborrowed, done: bun_jsc::Completion, ) -> Option> { this.value = Some(this.op.compute(&this.password)); diff --git a/src/runtime/dns_jsc/dns.rs b/src/runtime/dns_jsc/dns.rs index cb77f288f2af..3a30ac5cbb0b 100644 --- a/src/runtime/dns_jsc/dns.rs +++ b/src/runtime/dns_jsc/dns.rs @@ -1010,9 +1010,12 @@ pub mod get_addr_info_request { impl bun_jsc::JobContext for LibcLookup { type OffThread = Self; type Js = LibcRequest; + /// `getaddrinfo` waits on the resolver for as long as it likes; the + /// query is an owned copy. + type Vm = bun_jsc::Unborrowed; fn run( this: &mut Self, - _vm: &bun_jsc::vm_handle::Borrow, + _: &bun_jsc::Unborrowed, done: bun_jsc::Completion, ) -> Option> { this.backend.run(); diff --git a/src/runtime/image/Image.rs b/src/runtime/image/Image.rs index 7a02e6a4d7ff..f0699eb26c75 100644 --- a/src/runtime/image/Image.rs +++ b/src/runtime/image/Image.rs @@ -1465,6 +1465,7 @@ impl Drop for PendingTask { impl jsc::JobContext for PipelineTask { type OffThread = Self; type Js = PipelineJs; + type Vm = jsc::vm_handle::Borrow; fn run( this: &mut Self, _vm: &jsc::vm_handle::Borrow, diff --git a/src/runtime/node/node_crypto_binding.rs b/src/runtime/node/node_crypto_binding.rs index ac3c88150877..ae257a856f44 100644 --- a/src/runtime/node/node_crypto_binding.rs +++ b/src/runtime/node/node_crypto_binding.rs @@ -124,6 +124,7 @@ macro_rules! extern_crypto_job { impl JobContext for ExternJob { type OffThread = Self; type Js = Strong; + type Vm = Borrow; fn run( this: &mut Self, @@ -234,6 +235,7 @@ pub mod random { impl JobContext for RandomFillJob { type OffThread = Self; type Js = RandomFillJs; + type Vm = Borrow; fn run( this: &mut Self, @@ -1061,6 +1063,7 @@ mod _impl { impl JobContext for ScryptJob { type OffThread = Self; type Js = ScryptJs; + type Vm = Borrow; fn run( this: &mut Self, diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 6046c4ad2fd5..367583bdfd58 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -1251,6 +1251,9 @@ mod _async_tasks { { type OffThread = Self; type Js = AsyncFSJs; + /// `args` may hold the caller's JS buffers (a `Buffer` path, the data + /// of `write`, the destination of `read`), used by the dispatch below. + type Vm = bun_jsc::vm_handle::Borrow; fn run( this: &mut Self, @@ -2182,6 +2185,9 @@ mod _async_tasks { impl bun_jsc::JobContext for AsyncReaddirRecursiveTask { type OffThread = Self; type Js = AsyncFSJs; + /// Still carries its protected `args` (possibly a `Buffer` path), so it + /// keeps [`AsyncFSTask`]'s model. + type Vm = bun_jsc::vm_handle::Borrow; fn run( this: &mut Self, diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index eb149d320b7a..e1a5797be71c 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -3671,7 +3671,6 @@ impl BlobExt for Blob { } } - path_or_fd.to_thread_safe(); core::mem::replace( path_or_fd, PathOrFileDescriptor::Path(crate::webcore::node_types::PathLike::String( diff --git a/src/runtime/webcore/CompressionStreamCoder.rs b/src/runtime/webcore/CompressionStreamCoder.rs index 4f09b3658456..8753fda97478 100644 --- a/src/runtime/webcore/CompressionStreamCoder.rs +++ b/src/runtime/webcore/CompressionStreamCoder.rs @@ -833,6 +833,8 @@ pub struct CompressionAsyncJs { impl bun_jsc::JobContext for CompressionAsyncCtx { type OffThread = Self; type Js = CompressionAsyncJs; + /// `input` may be the pinned chunk's JS bytes, read below. + type Vm = bun_jsc::vm_handle::Borrow; fn run( this: &mut Self, diff --git a/src/runtime/webcore/blob/Store.rs b/src/runtime/webcore/blob/Store.rs index 35c6943d07d0..c616ece14ce8 100644 --- a/src/runtime/webcore/blob/Store.rs +++ b/src/runtime/webcore/blob/Store.rs @@ -18,7 +18,7 @@ use crate::webcore::s3::client::{ S3Credentials, S3CredentialsWithOptions, S3DeleteResult, S3ListObjectsOptions, S3ListObjectsResult, }; -use bun_core::{ZigString, strings}; +use bun_core::{ZigString, ZigStringSlice, strings}; use bun_http_types::MimeType::MimeType; use bun_url::URL; @@ -110,6 +110,21 @@ fn mime_from_path_ext(sliced: &[u8]) -> Option { bun_http_types::MimeType::by_extension_no_default(ext) } +/// Make `path` the store's own. The pool jobs reading a store (`CopyFile`, +/// `ReadFile`, `WriteFile`) run without a VM borrow and may drop its last ref +/// after the VM is gone, so it may keep nothing of the JS heap: a `Buffer` path +/// (`Bun.file(bytes)`, documented as copying) points into the caller's +/// ArrayBuffer and is copied out here, on the JS thread, which also releases +/// the pin on that ArrayBuffer; a string path gets its own WTF impl. +fn own_path(path: &mut PathLike) { + if let PathLike::Buffer(buffer) = &*path { + let copy = bun_core::handle_oom(ZigStringSlice::init_dupe(buffer.slice())); + *path = PathLike::EncodedSlice(copy); + return; + } + path.to_thread_safe(); +} + impl StoreExt for Store { /// Caller is responsible for derefing the Store. fn to_any_blob(&mut self) -> Option { @@ -128,8 +143,7 @@ impl StoreExt for Store { credentials: S3Credentials, ) -> Result, crate::Error> { let mut path = pathlike; - // this actually protects/refs the pathlike - path.to_thread_safe(); + own_path(&mut path); // Compute the extension-derived fallback before moving `path` into the // Store so we don't need to clone the owned PathLike. @@ -144,9 +158,13 @@ impl StoreExt for Store { } fn init_file( - pathlike: PathOrFileDescriptor, + mut pathlike: PathOrFileDescriptor, mime_type: Option, ) -> Result, crate::Error> { + if let PathOrFileDescriptor::Path(path) = &mut pathlike { + own_path(path); + } + // Compute the extension-derived fallback before moving `pathlike` into // the Store so we don't need to clone the owned PathOrFileDescriptor. let mime_type = mime_type.or_else(|| match &pathlike { diff --git a/src/runtime/webcore/blob/copy_file.rs b/src/runtime/webcore/blob/copy_file.rs index a4b016417992..f963ba13d810 100644 --- a/src/runtime/webcore/blob/copy_file.rs +++ b/src/runtime/webcore/blob/copy_file.rs @@ -75,9 +75,13 @@ unsafe impl Send for CopyFile {} impl jsc::JobContext for CopyFile { type OffThread = Self; type Js = jsc::JSPromiseStrong; + /// The whole copy happens in `run`, and the source is opened blocking: a + /// FIFO, tty or pipe source (`Bun.write(Bun.stdout, Bun.stdin)`) sits there + /// until the other side acts. Everything read meanwhile is the stores'. + type Vm = jsc::Unborrowed; fn run( this: &mut Self, - _vm: &jsc::vm_handle::Borrow, + _: &jsc::Unborrowed, done: bun_jsc::Completion, ) -> Option> { this.run_async(); diff --git a/src/runtime/webcore/blob/read_file.rs b/src/runtime/webcore/blob/read_file.rs index fefb816d9ecd..40da5a08455f 100644 --- a/src/runtime/webcore/blob/read_file.rs +++ b/src/runtime/webcore/blob/read_file.rs @@ -224,9 +224,12 @@ impl bun_jsc::JobContext for ReadFile { /// Where the bytes go: completed by `then`, or cancelled when the VM releases the JS sides of /// its live jobs at teardown (a refused or unrun read then frees only this off-thread part). type Js = ReadFileCompletionFns; + /// Reads only the stores' own state (see the `Send` note above); a regular + /// file is read to the end inside `run`. + type Vm = bun_jsc::Unborrowed; fn run( this: &mut Self, - _vm: &bun_jsc::vm_handle::Borrow, + _: &bun_jsc::Unborrowed, done: bun_jsc::Completion, ) -> Option> { // Starts the read; finishes from the io loop via the token. diff --git a/src/runtime/webcore/blob/write_file.rs b/src/runtime/webcore/blob/write_file.rs index 293d064020a5..e607ee97a9ab 100644 --- a/src/runtime/webcore/blob/write_file.rs +++ b/src/runtime/webcore/blob/write_file.rs @@ -48,9 +48,11 @@ impl bun_jsc::JobContext for WriteFile { type OffThread = Self; /// The completion is delivered through `on_complete_callback(ctx, ..)`. type Js = (); + /// Writes the source store's own bytes (see the `Send` note above). + type Vm = bun_jsc::Unborrowed; fn run( this: &mut Self, - _vm: &bun_jsc::vm_handle::Borrow, + _: &bun_jsc::Unborrowed, done: bun_jsc::Completion, ) -> Option> { // Starts the write; finishes from the io loop via the token. diff --git a/test/js/bun/util/bun-file.test.ts b/test/js/bun/util/bun-file.test.ts index 6a422f38589f..c8fad1b680b6 100644 --- a/test/js/bun/util/bun-file.test.ts +++ b/test/js/bun/util/bun-file.test.ts @@ -155,3 +155,29 @@ test("Bun.file().json() with UTF-8 BOM does not free an interior pointer", async }); expect(exitCode).toBe(0); }); + +// The store behind a file blob is read (and may be released) on pool threads +// after the call that made it returns, so it has to own its path: a byte path is +// copied out of the caller's buffer, as documented, not read through it later. +test.each([ + ["Uint8Array", (bytes: Uint8Array) => bytes], + ["ArrayBuffer", (bytes: Uint8Array) => bytes.buffer], +])("Bun.file() and Bun.write() copy a %s path out of the buffer", async (_, asArgument) => { + await using dir = tempDir("bun-file-byte-path", { "a.txt": "from a", "z.txt": "from z" }); + const encode = (name: string) => new TextEncoder().encode(join(String(dir), name)); + // ".txt" -> "z.txt", in place. + const retarget = (bytes: Uint8Array) => void (bytes[bytes.length - 5] = "z".charCodeAt(0)); + + const source = encode("a.txt"); + const file = Bun.file(asArgument(source)); + retarget(source); + expect(file.name).toBe(join(String(dir), "a.txt")); + expect(await file.text()).toBe("from a"); + + const destination = encode("c.txt"); + const written = Bun.write(asArgument(destination), file); + retarget(destination); + expect(await written).toBe(6); + expect(await Bun.file(join(String(dir), "c.txt")).text()).toBe("from a"); + expect(await Bun.file(join(String(dir), "z.txt")).text()).toBe("from z"); +}); diff --git a/test/js/web/workers/worker-refused-completion.test.ts b/test/js/web/workers/worker-refused-completion.test.ts index 20936d3485ed..be458884d757 100644 --- a/test/js/web/workers/worker-refused-completion.test.ts +++ b/test/js/web/workers/worker-refused-completion.test.ts @@ -41,6 +41,14 @@ const ROWS: Row[] = [ worker: `Bun.file(process.execPath).slice(0, 65536).text();`, refused: "blob::read_file::ReadFile", }, + { + // The whole copy runs on the pool without holding the worker's VM, so a + // copy still going when the worker exits is the normal way to get here. + name: "Bun.write(file, file)", + worker: `Bun.write("/dev/null", Bun.file(process.execPath).slice(0, 65536));`, + refused: "blob::copy_file::CopyFile", + skip: isWindows, + }, { // Same read job, different completion: the image's read chain is handed // ECANCELED at teardown and has to free itself. diff --git a/test/js/web/workers/worker-terminate-lifetime.test.ts b/test/js/web/workers/worker-terminate-lifetime.test.ts index 4e9376f17fbc..0faba27dc179 100644 --- a/test/js/web/workers/worker-terminate-lifetime.test.ts +++ b/test/js/web/workers/worker-terminate-lifetime.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from "bun:test"; -import { bunEnv, bunExe, isASAN, isDebug, tempDir, tls } from "harness"; +import { bunEnv, bunExe, isASAN, isDebug, isWindows, tempDir, tls } from "harness"; +import { mkfifo } from "mkfifo"; import { join } from "path"; // Worker VM startup/teardown is much slower under debug and/or ASAN; these @@ -521,3 +522,45 @@ test( }, timeout, ); + +// Regression: a worker's Bun.write(file, file) runs the whole copy on the thread +// pool and opens the source blocking, so with a FIFO (or tty / idle pipe) source +// the pool thread sits in open(2) until the other side shows up. The worker's +// teardown waited for that job, so terminate() never settled. Nothing ever +// opens the FIFO's other end here: the copy stays blocked for the whole test and +// terminate() has to settle without it. +test.skipIf(isWindows)( + "terminate() settles while the worker's Bun.write(file, file) is blocked opening a FIFO", + async () => { + using dir = tempDir("worker-terminate-copyfile-fifo", { + "main.cjs": ` + const { Worker, isMainThread, parentPort } = require("node:worker_threads"); + const fs = require("node:fs"); + const path = require("node:path"); + if (isMainThread) { + const w = new Worker(__filename); + w.on("message", async () => { + await w.terminate(); + console.log("terminated"); + }); + } else { + Bun.write(path.join(__dirname, "out"), Bun.file(path.join(__dirname, "fifo"))).catch(() => {}); + // The copy was queued first; once a pool job queued after it has come + // back, the pool has taken the copy and it is blocked in open(2). + fs.promises.stat(__filename).then(() => parentPort.postMessage("blocked")); + } + `, + }); + mkfifo(join(String(dir), "fifo"), 0o600); + await using proc = Bun.spawn({ + cmd: [bunExe(), "main.cjs"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ stdout: "terminated\n", stderr: "", exitCode: 0 }); + }, + timeout, +); From 056afb765765399ab8e3ac7f96d861f8dc1790d8 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 01:26:04 +0000 Subject: [PATCH 2/4] test: do not assert the copy's byte count, which Windows reports as 0 (#33715) --- test/js/bun/util/bun-file.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/js/bun/util/bun-file.test.ts b/test/js/bun/util/bun-file.test.ts index c8fad1b680b6..af52d212df04 100644 --- a/test/js/bun/util/bun-file.test.ts +++ b/test/js/bun/util/bun-file.test.ts @@ -177,7 +177,9 @@ test.each([ const destination = encode("c.txt"); const written = Bun.write(asArgument(destination), file); retarget(destination); - expect(await written).toBe(6); + // The resolved byte count is not asserted: the Windows file-to-file copy + // reports 0 (#33715); where the bytes went is what this test is about. + await written; expect(await Bun.file(join(String(dir), "c.txt")).text()).toBe("from a"); expect(await Bun.file(join(String(dir), "z.txt")).text()).toBe("from z"); }); From e61cb3df231e28deee18bd31fa833fa53205f9cd Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 01:40:37 +0000 Subject: [PATCH 3/4] Shorten the new comments to one line each --- src/jsc/JSSecrets.rs | 3 +- src/jsc/VmHandle.rs | 8 ++--- src/jsc/bindings/JSSecrets.cpp | 3 +- src/jsc/job.rs | 47 ++++++++------------------- src/runtime/api/Archive.rs | 5 ++- src/runtime/dns_jsc/dns.rs | 3 +- src/runtime/node/node_fs.rs | 6 ++-- src/runtime/webcore/blob/Store.rs | 8 ++--- src/runtime/webcore/blob/copy_file.rs | 4 +-- src/runtime/webcore/blob/read_file.rs | 3 +- 10 files changed, 27 insertions(+), 63 deletions(-) diff --git a/src/jsc/JSSecrets.rs b/src/jsc/JSSecrets.rs index 406fd2bce10b..4722287983e9 100644 --- a/src/jsc/JSSecrets.rs +++ b/src/jsc/JSSecrets.rs @@ -37,8 +37,7 @@ pub(crate) struct SecretsJob { impl crate::JobContext for SecretsJob { type OffThread = Self; type Js = Strong; - /// The credential store (Keychain, libsecret's D-Bus service, ...) may sit - /// on a user prompt indefinitely; the job owns everything it hands it. + /// The credential store may sit on a user prompt indefinitely; the job owns all it hands it. type Vm = crate::Unborrowed; fn run( diff --git a/src/jsc/VmHandle.rs b/src/jsc/VmHandle.rs index 67586ef49e4e..3539eb8a751a 100644 --- a/src/jsc/VmHandle.rs +++ b/src/jsc/VmHandle.rs @@ -247,8 +247,7 @@ impl VmHandle { /// This job is about to use VM-owned memory off-thread; `None` if the VM /// is closed (touch nothing). Hold the result until done. Jobs that could - /// block indefinitely on an external party must own their memory instead - /// and only ask [`is_closed`](Self::is_closed) whether to bother starting. + /// block indefinitely on an external party must own their memory instead. pub fn borrow(&self) -> Option { let a = self.enter()?; // SAFETY: lifetime extension is sound because `Borrow` also holds a @@ -268,10 +267,7 @@ impl VmHandle { (self.0.hot.state.load(Ordering::SeqCst) == State::Open as u8).then_some(b) } - /// Whether `close()` has run, i.e. whatever this work would post is going - /// to be refused. Any thread; a snapshot, so only for deciding not to start - /// work that owns its memory — work that uses the VM's takes a - /// [`borrow`](Self::borrow). + /// Whether `close()` has run (anything posted from now on is refused). Any thread; a snapshot. pub fn is_closed(&self) -> bool { self.0.hot.state.load(Ordering::SeqCst) == State::Closed as u8 } diff --git a/src/jsc/bindings/JSSecrets.cpp b/src/jsc/bindings/JSSecrets.cpp index c8d571d83896..ae61c1ea6a86 100644 --- a/src/jsc/bindings/JSSecrets.cpp +++ b/src/jsc/bindings/JSSecrets.cpp @@ -242,8 +242,7 @@ struct SecretsJobOptions { // C interface implementation for the native binding extern "C" { -// Runs on the threadpool - does the actual platform API work. The job's VM may -// be torn down while this blocks (a keychain prompt), so it gets nothing of it. +// Runs on the threadpool - does the actual platform API work void Bun__SecretsJobOptions__runTask(SecretsJobOptions* opts) { // Already have CString fields, pass them directly to platform APIs diff --git a/src/jsc/job.rs b/src/jsc/job.rs index 6d385ac678d1..b9b297204e57 100644 --- a/src/jsc/job.rs +++ b/src/jsc/job.rs @@ -4,14 +4,11 @@ //! [`WorkPool`], and completes on the JS thread again — unless its VM went away //! meanwhile. Which thread may touch which part of it is in the types: //! -//! * [`JobContext::OffThread`] is what the pool body sees. It is `Send`, and a -//! body never starts against a VM that is already closed. Whether the VM's -//! teardown also *waits* for a body that is mid-flight is the impl's -//! [`JobContext::Vm`]: a body that reads VM-owned memory runs under a VM -//! [`Borrow`] the carrier takes for it (JS-backed memory is reachable only -//! through [`JsPtr`], i.e. only while that borrow, or a [`JsThread`], is in -//! hand); a body that owns everything it touches runs [`Unborrowed`], and a -//! VM torn down underneath it simply refuses its completion. +//! * [`JobContext::OffThread`] is what the pool body sees. It is `Send`, and it +//! runs under a VM [`Borrow`] if its [`JobContext::Vm`] says so; teardown then +//! waits for a body that is mid-flight and a body never starts against a VM +//! that is already closed. JS-backed memory it needs is reachable only through +//! [`JsPtr`], i.e. only while that borrow (or a [`JsThread`]) is in hand. //! * [`JobContext::Js`] is the completion's JS-thread state (promise, callback, //! wrapper refs, pins, protected buffers). It is [`JsAffine`] and lives in a //! [`JsSide`], which opens only with a [`JsThread`] token and is never dropped @@ -237,26 +234,14 @@ pub trait JobContext: Sized + 'static { type OffThread: Send; type Js: JsAffine; - /// What [`run`](Self::run) holds on the VM while it executes: - /// - /// * [`Borrow`] — `OffThread` reaches memory the VM owns (a [`JsPtr`], the - /// bytes of a pinned buffer), so the body runs under a VM borrow and the - /// VM's teardown waits for it. Only for bodies that cannot block on an - /// external party: `terminate()` of the worker waits exactly as long. - /// * [`Unborrowed`] — `OffThread` owns everything the body touches, so - /// teardown does not wait for it: a body still running when its VM goes - /// away (a copy blocked on a FIFO, `getaddrinfo`) finishes on its own - /// time and its completion is refused, i.e. for such a job the release on - /// the pool thread ([`Postable::release_refused`]) is the normal end of - /// in-flight work at teardown, not a rare race. - /// - /// [`Postable::release_refused`]: crate::Postable::release_refused + /// [`Borrow`] if `OffThread` reaches VM-owned memory during `run`, else [`Unborrowed`]. type Vm: VmHold; - /// Pool thread, with [`Self::Vm`] held for the whole call. Return `done` to - /// complete now; keep it (e.g. across async I/O that finishes on another - /// thread) and call [`Completion::finish`] later to complete then. Work - /// that outlives this call runs under no borrow and must touch only `off`. + /// Pool thread, with [`Self::Vm`] held by the carrier for the whole call. + /// Return `done` to complete now; keep it (e.g. across async I/O that + /// finishes on another thread) and call [`Completion::finish`] later to + /// complete then. Work that outlives this call runs under no borrow and + /// must touch only `off`. fn run( off: &mut Self::OffThread, vm: &Self::Vm, @@ -274,11 +259,9 @@ mod sealed { impl Sealed for super::Unborrowed {} } -/// What the carrier holds on the VM while [`JobContext::run`] executes: see -/// [`JobContext::Vm`]. Implemented by [`Borrow`] and [`Unborrowed`] only. +/// The two things [`JobContext::run`] can hold on the VM: [`Borrow`] or [`Unborrowed`]. pub trait VmHold: sealed::Sealed + Sized { - /// Pool thread, before the body: `None` if the VM is already closed, in - /// which case the body does not run. + /// `None` once the VM is closed, in which case the body does not run. fn acquire(handle: &LoopHandle) -> Option; } @@ -289,9 +272,7 @@ impl VmHold for Borrow { } } -/// The [`JobContext::Vm`] of a job whose off-thread part owns everything its -/// body touches: nothing is held, so the VM's teardown does not wait for the -/// body. +/// The [`JobContext::Vm`] of a body that owns everything it touches: teardown does not wait for it. pub struct Unborrowed(()); impl VmHold for Unborrowed { diff --git a/src/runtime/api/Archive.rs b/src/runtime/api/Archive.rs index 5c8575eddae3..f807a25fec8b 100644 --- a/src/runtime/api/Archive.rs +++ b/src/runtime/api/Archive.rs @@ -674,9 +674,7 @@ impl PromiseResult { /// One `Bun.Archive` operation's pool-side work: `run` on the thread pool /// stores its result on `self`; `run_from_js` turns it into the promise's -/// value. It is the off-thread part of an `AsyncTask` job, which runs -/// [`Unborrowed`](bun_jsc::Unborrowed): a context owns everything `run` -/// touches (a store ref, copied arguments), never JS memory. +/// value. It is the off-thread part of an `AsyncTask` job. pub trait TaskContext: Send + 'static { /// Runs on thread pool. Stores its result on `self`. fn run(&mut self); @@ -689,6 +687,7 @@ pub struct AsyncTask(core::marker::PhantomData); impl bun_jsc::JobContext for AsyncTask { type OffThread = C; type Js = JSPromiseStrong; + /// A context owns everything `run` touches: a store ref and copied arguments. type Vm = bun_jsc::Unborrowed; fn run( ctx: &mut C, diff --git a/src/runtime/dns_jsc/dns.rs b/src/runtime/dns_jsc/dns.rs index 3a30ac5cbb0b..bc520865dee0 100644 --- a/src/runtime/dns_jsc/dns.rs +++ b/src/runtime/dns_jsc/dns.rs @@ -1010,8 +1010,7 @@ pub mod get_addr_info_request { impl bun_jsc::JobContext for LibcLookup { type OffThread = Self; type Js = LibcRequest; - /// `getaddrinfo` waits on the resolver for as long as it likes; the - /// query is an owned copy. + /// `getaddrinfo` waits on the resolver as long as it likes; the query is an owned copy. type Vm = bun_jsc::Unborrowed; fn run( this: &mut Self, diff --git a/src/runtime/node/node_fs.rs b/src/runtime/node/node_fs.rs index 367583bdfd58..82d34230853d 100644 --- a/src/runtime/node/node_fs.rs +++ b/src/runtime/node/node_fs.rs @@ -1251,8 +1251,7 @@ mod _async_tasks { { type OffThread = Self; type Js = AsyncFSJs; - /// `args` may hold the caller's JS buffers (a `Buffer` path, the data - /// of `write`, the destination of `read`), used by the dispatch below. + /// `args` may alias the caller's buffers (`Buffer` paths, `write` data, `read` targets). type Vm = bun_jsc::vm_handle::Borrow; fn run( @@ -2185,8 +2184,7 @@ mod _async_tasks { impl bun_jsc::JobContext for AsyncReaddirRecursiveTask { type OffThread = Self; type Js = AsyncFSJs; - /// Still carries its protected `args` (possibly a `Buffer` path), so it - /// keeps [`AsyncFSTask`]'s model. + /// Still carries its protected `args` (possibly a `Buffer` path), like [`AsyncFSTask`]. type Vm = bun_jsc::vm_handle::Borrow; fn run( diff --git a/src/runtime/webcore/blob/Store.rs b/src/runtime/webcore/blob/Store.rs index c616ece14ce8..7f26756c548b 100644 --- a/src/runtime/webcore/blob/Store.rs +++ b/src/runtime/webcore/blob/Store.rs @@ -110,14 +110,10 @@ fn mime_from_path_ext(sliced: &[u8]) -> Option { bun_http_types::MimeType::by_extension_no_default(ext) } -/// Make `path` the store's own. The pool jobs reading a store (`CopyFile`, -/// `ReadFile`, `WriteFile`) run without a VM borrow and may drop its last ref -/// after the VM is gone, so it may keep nothing of the JS heap: a `Buffer` path -/// (`Bun.file(bytes)`, documented as copying) points into the caller's -/// ArrayBuffer and is copied out here, on the JS thread, which also releases -/// the pin on that ArrayBuffer; a string path gets its own WTF impl. +/// File jobs read and release stores without a VM borrow, so a store keeps nothing of the JS heap. fn own_path(path: &mut PathLike) { if let PathLike::Buffer(buffer) = &*path { + // Documented as a copy; dropping the `Buffer` below unpins it, here on the JS thread. let copy = bun_core::handle_oom(ZigStringSlice::init_dupe(buffer.slice())); *path = PathLike::EncodedSlice(copy); return; diff --git a/src/runtime/webcore/blob/copy_file.rs b/src/runtime/webcore/blob/copy_file.rs index f963ba13d810..6d947da75bf6 100644 --- a/src/runtime/webcore/blob/copy_file.rs +++ b/src/runtime/webcore/blob/copy_file.rs @@ -75,9 +75,7 @@ unsafe impl Send for CopyFile {} impl jsc::JobContext for CopyFile { type OffThread = Self; type Js = jsc::JSPromiseStrong; - /// The whole copy happens in `run`, and the source is opened blocking: a - /// FIFO, tty or pipe source (`Bun.write(Bun.stdout, Bun.stdin)`) sits there - /// until the other side acts. Everything read meanwhile is the stores'. + /// Blocks for the whole copy (a FIFO source waits for a writer); touches only store state. type Vm = jsc::Unborrowed; fn run( this: &mut Self, diff --git a/src/runtime/webcore/blob/read_file.rs b/src/runtime/webcore/blob/read_file.rs index 40da5a08455f..235796a9e696 100644 --- a/src/runtime/webcore/blob/read_file.rs +++ b/src/runtime/webcore/blob/read_file.rs @@ -224,8 +224,7 @@ impl bun_jsc::JobContext for ReadFile { /// Where the bytes go: completed by `then`, or cancelled when the VM releases the JS sides of /// its live jobs at teardown (a refused or unrun read then frees only this off-thread part). type Js = ReadFileCompletionFns; - /// Reads only the stores' own state (see the `Send` note above); a regular - /// file is read to the end inside `run`. + /// Touches only the stores' own state (see the `Send` note above). type Vm = bun_jsc::Unborrowed; fn run( this: &mut Self, From 3a626be7382a57bbc337fc3f935e203a408ea344 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 14 Aug 2026 01:46:35 +0000 Subject: [PATCH 4/4] Say why the image pipeline job keeps the borrow --- src/runtime/image/Image.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/runtime/image/Image.rs b/src/runtime/image/Image.rs index f0699eb26c75..2ed8d1f05f60 100644 --- a/src/runtime/image/Image.rs +++ b/src/runtime/image/Image.rs @@ -1465,6 +1465,7 @@ impl Drop for PendingTask { impl jsc::JobContext for PipelineTask { type OffThread = Self; type Js = PipelineJs; + /// `input` borrows a pinned JS buffer or the `Image`'s own bytes (see the `Send` note above). type Vm = jsc::vm_handle::Borrow; fn run( this: &mut Self,