From 9defe269252027d34da8c3eb9cd9635f42d9e824 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 6 Aug 2026 20:49:12 +0000 Subject: [PATCH 1/4] runtime: report failed promise settles in event-loop completions The subprocess exit path, ByteStream cancel, Bun.write completions (copy_file, write_file, and the locked-body wait task), S3 multipart buffering, and the valkey timeout timers all discarded the result of settling a promise (or of a completion callback) with `let _ =`. A failed settle leaves a JS exception pending on the VM, and these run as event-loop completions with no host-call boundary to surface it, so the stale exception rode the tick into unrelated JS: debug/ASAN builds hit JSC's exception-state assertions and release builds misattribute the error. Check the settle result instead and report a pending non-termination exception through the VM's unhandled-exception path, the same idiom the socket, IPC, and http2 completions use. The error label cannot be trusted to distinguish a thrown exception from termination (the FFI wrappers collapse both to the terminated sentinel), so the VM state decides: termination exceptions stay pending as the event loop expects. Tests pin the touched completions (subprocess exit-code and signal settles, locked-body and file-to-file Bun.write) under a hostile Object.prototype.then accessor, the prototype-pollution state the fuzzer runs scripts in. --- src/runtime/api/bun/subprocess.rs | 37 ++++++++-------- src/runtime/valkey_jsc/js_valkey.rs | 13 ++++-- src/runtime/webcore/ByteStream.rs | 16 ++++--- src/runtime/webcore/blob/copy_file.rs | 15 ++++++- src/runtime/webcore/blob/write_file.rs | 41 ++++++++++++++---- src/runtime/webcore/s3/multipart.rs | 17 ++++++-- test/js/bun/io/bun-write.test.js | 59 ++++++++++++++++++++++++++ test/js/bun/spawn/spawn.test.ts | 37 ++++++++++++++++ 8 files changed, 194 insertions(+), 41 deletions(-) diff --git a/src/runtime/api/bun/subprocess.rs b/src/runtime/api/bun/subprocess.rs index 1e17989a5791..0232cf3e6ce3 100644 --- a/src/runtime/api/bun/subprocess.rs +++ b/src/runtime/api/bun/subprocess.rs @@ -1119,34 +1119,35 @@ impl Subprocess<'_> { did_update_has_pending_activity = true; } - match status { - Status::Exited(exited) => { - let _ = promise - .as_any_promise() - .unwrap() - .resolve(global_this, JSValue::js_number(exited.code as f64)); - // TODO: properly propagate exception upwards - } + let settled = match status { + Status::Exited(exited) => promise + .as_any_promise() + .unwrap() + .resolve(global_this, JSValue::js_number(exited.code as f64)), Status::Err(err) => { let js_err = err.to_js(global_this); - let _ = promise + promise .as_any_promise() .unwrap() - .reject_with_async_stack(global_this, js_err); - // TODO: properly propagate exception upwards - } - Status::Signaled(signaled) => { - let _ = promise.as_any_promise().unwrap().resolve( - global_this, - JSValue::js_number(128u8.wrapping_add(*signaled) as f64), - ); - // TODO: properly propagate exception upwards + .reject_with_async_stack(global_this, js_err) } + Status::Signaled(signaled) => promise.as_any_promise().unwrap().resolve( + global_this, + JSValue::js_number(128u8.wrapping_add(*signaled) as f64), + ), _ => { // crash in debug mode #[cfg(debug_assertions)] unreachable!(); + #[cfg(not(debug_assertions))] + Ok(()) } + }; + if settled.is_err() { + // A failed settle leaves an exception pending on the VM; + // the exit callback below must not run with it (the + // error label erases Thrown, so probe the VM instead). + global_this.report_active_exception_as_unhandled(jsc::JsError::Thrown); } } diff --git a/src/runtime/valkey_jsc/js_valkey.rs b/src/runtime/valkey_jsc/js_valkey.rs index 8c68f4c82ed3..c9bfab23525c 100644 --- a/src/runtime/valkey_jsc/js_valkey.rs +++ b/src/runtime/valkey_jsc/js_valkey.rs @@ -1165,8 +1165,11 @@ impl JSValkeyClient { .expect("unreachable"); let len = start - cur.len(); let msg = &buf[..len]; - let _ = self.client_fail(msg, protocol::RedisError::IdleTimeout); - // TODO: properly propagate exception upwards + if let Err(e) = self.client_fail(msg, protocol::RedisError::IdleTimeout) { + // A failed settle inside `fail` leaves an exception pending + // on the VM; this timer callback has nowhere to bubble it. + self.global_object.report_active_exception_as_unhandled(e); + } } valkey::Status::Disconnected | valkey::Status::Connecting => { use std::io::Write; @@ -1180,8 +1183,10 @@ impl JSValkeyClient { .expect("unreachable"); let len = start - cur.len(); let msg = &buf[..len]; - let _ = self.client_fail(msg, protocol::RedisError::ConnectionTimeout); - // TODO: properly propagate exception upwards + if let Err(e) = self.client_fail(msg, protocol::RedisError::ConnectionTimeout) { + // See the idle-timeout arm above. + self.global_object.report_active_exception_as_unhandled(e); + } } } } diff --git a/src/runtime/webcore/ByteStream.rs b/src/runtime/webcore/ByteStream.rs index a696aace9e38..d010f8110bc3 100644 --- a/src/runtime/webcore/ByteStream.rs +++ b/src/runtime/webcore/ByteStream.rs @@ -620,11 +620,17 @@ impl ByteStream { if let Some(mut action) = self.buffer_action.replace(None) { let global = self.parent_const().global_this(); - // TODO: properly propagate exception upwards - let _ = action.reject( - global, - &streams::StreamError::AbortReason(jsc::CommonAbortReason::UserAbort), - ); + if action + .reject( + global, + &streams::StreamError::AbortReason(jsc::CommonAbortReason::UserAbort), + ) + .is_err() + { + // A failed settle leaves an exception pending on the VM (the + // error label erases Thrown, so probe the VM instead). + global.report_active_exception_as_unhandled(jsc::JsError::Thrown); + } self.buffer_action.set(None); } } diff --git a/src/runtime/webcore/blob/copy_file.rs b/src/runtime/webcore/blob/copy_file.rs index cf4aae0668e7..4e2c40e6b7e2 100644 --- a/src/runtime/webcore/blob/copy_file.rs +++ b/src/runtime/webcore/blob/copy_file.rs @@ -1642,7 +1642,12 @@ impl<'a> CopyFileWindows<'a> { // SAFETY: self was heap-allocated in init(); destroy reclaims and drops it. self is not accessed afterward. unsafe { Self::destroy(core::ptr::from_mut(self)) }; // `promise` points to a GC-owned `JSPromise` cell, not into `self`; valid after `destroy`. - let _ = promise.reject(global_this, err_instance); // TODO: properly propagate exception upwards + if promise.reject(global_this, err_instance).is_err() { + // A failed settle leaves an exception pending on the VM; report it + // so it cannot ride the tick into unrelated JS (the error label + // erases Thrown, so probe the VM instead). + global_this.report_active_exception_as_unhandled(jsc::JsError::Thrown); + } } pub(crate) fn on_complete(&mut self, written_actual: usize) { @@ -1729,7 +1734,13 @@ impl<'a> CopyFileWindows<'a> { // SAFETY: self was heap-allocated in init(); destroy reclaims and drops it. self is not accessed afterward. unsafe { Self::destroy(core::ptr::from_mut(self)) }; // `promise` points to a GC-owned `JSPromise` cell, not into `self`; valid after `destroy`. - let _ = promise.resolve(global_this, JSValue::js_number_from_uint64(written as u64)); // TODO: properly propagate exception upwards + if promise + .resolve(global_this, JSValue::js_number_from_uint64(written as u64)) + .is_err() + { + // See `throw` above: keep a failed settle's exception off the tick. + global_this.report_active_exception_as_unhandled(jsc::JsError::Thrown); + } } #[cold] diff --git a/src/runtime/webcore/blob/write_file.rs b/src/runtime/webcore/blob/write_file.rs index 32a0e2762a8c..73579a43ede1 100644 --- a/src/runtime/webcore/blob/write_file.rs +++ b/src/runtime/webcore/blob/write_file.rs @@ -927,7 +927,8 @@ mod windows_impl { ) } { WriteFileWindowsError::WriteFileWindowsDeinitialized => {} - WriteFileWindowsError::JSTerminated => {} // TODO: properly propagate exception upwards + // `run_from_js_thread` already reported the failed settle. + WriteFileWindowsError::JSTerminated => {} } return; } @@ -940,7 +941,8 @@ mod windows_impl { if let Err(e) = unsafe { Self::do_write_loop(this, (*this).loop_()) } { match e { WriteFileWindowsError::WriteFileWindowsDeinitialized => {} - WriteFileWindowsError::JSTerminated => {} // TODO: properly propagate exception upwards + // `run_from_js_thread` already reported the failed settle. + WriteFileWindowsError::JSTerminated => {} } } } @@ -989,7 +991,8 @@ mod windows_impl { // SAFETY: caller contract — `this` is live; `throw` consumes it. match unsafe { Self::throw(this, err_) } { WriteFileWindowsError::WriteFileWindowsDeinitialized => {} - WriteFileWindowsError::JSTerminated => {} // TODO: properly propagate exception upwards + // `run_from_js_thread` already reported the failed settle. + WriteFileWindowsError::JSTerminated => {} } return; } @@ -998,14 +1001,15 @@ mod windows_impl { if let Err(e) = unsafe { Self::open(this) } { match e { WriteFileWindowsError::WriteFileWindowsDeinitialized => {} - WriteFileWindowsError::JSTerminated => {} // TODO: properly propagate exception upwards + // `run_from_js_thread` already reported the failed settle. + WriteFileWindowsError::JSTerminated => {} } } } /// `ManagedTask`-shaped trampoline for [`on_mkdirp_complete`]: takes /// `*mut Self` and returns the event-loop `JsResult<()>` (always `Ok`; - /// the inner body already swallows `JSTerminated`). + /// a failed settle is reported inside `run_from_js_thread`). fn on_mkdirp_complete_task(this: *mut WriteFileWindows) -> bun_event_loop::JsResult<()> { // SAFETY: `this` is the live Box-allocated `WriteFileWindows` whose // pointer was stashed in `on_mkdirp_complete_concurrent` below; @@ -1059,7 +1063,8 @@ mod windows_impl { ) } { WriteFileWindowsError::WriteFileWindowsDeinitialized => {} - WriteFileWindowsError::JSTerminated => {} // TODO: properly propagate exception upwards + // `run_from_js_thread` already reported the failed settle. + WriteFileWindowsError::JSTerminated => {} } return; } @@ -1070,7 +1075,8 @@ mod windows_impl { if let Err(e) = unsafe { Self::do_write_loop(this, (*this).loop_()) } { match e { WriteFileWindowsError::WriteFileWindowsDeinitialized => {} - WriteFileWindowsError::JSTerminated => {} // TODO: properly propagate exception upwards + // `run_from_js_thread` already reported the failed settle. + WriteFileWindowsError::JSTerminated => {} } } } @@ -1096,12 +1102,19 @@ mod windows_impl { // SAFETY: caller contract — `this` is live; copy out everything we // need before `deinit` frees the allocation. let (cb, cb_ctx) = unsafe { ((*this).on_complete_callback, (*this).on_complete_ctx) }; + // SAFETY: caller contract — `this` is live; the VM-owned event + // loop (and its global) outlives the request. + let global = unsafe { (*(*this).event_loop).global_ref() }; // 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))) { + // A failed settle leaves an exception pending on the VM; + // report it so it cannot ride the tick into unrelated JS + // (the error label erases Thrown, so probe the VM instead). + global.report_active_exception_as_unhandled(jsc::JsError::Thrown); return e.into(); } } else { @@ -1110,6 +1123,8 @@ mod windows_impl { // SAFETY: caller contract — `this` is live; consumed here. unsafe { Self::deinit(this) }; if let Err(e) = cb(cb_ctx, WriteFileResultType::Result(wrote as SizeType)) { + // See the error arm above. + global.report_active_exception_as_unhandled(jsc::JsError::Thrown); return e.into(); } } @@ -1343,8 +1358,16 @@ impl WriteFileWaitFromLockedValueTask { let this = unsafe { bun_core::heap::take(this.cast::().as_ptr()) }; - let _ = Self::then(this, value); - // TODO: properly propagate exception upwards + // `BackRef` is `Copy`; keep the global past the consuming call. + let global_ref = this.global_this; + if Self::then(this, value).is_err() { + // A failed settle leaves an exception pending on the VM; report it + // so it cannot ride the tick into unrelated JS (the error label + // erases Thrown, so probe the VM instead). + global_ref + .get() + .report_active_exception_as_unhandled(jsc::JsError::Thrown); + } } pub(crate) fn then( diff --git a/src/runtime/webcore/s3/multipart.rs b/src/runtime/webcore/s3/multipart.rs index 429fac114065..3e4466945aa9 100644 --- a/src/runtime/webcore/s3/multipart.rs +++ b/src/runtime/webcore/s3/multipart.rs @@ -990,7 +990,7 @@ impl MultiPartUpload { self.state.set(State::SinglefileStarted); // we can do only 1 request self.ref_(); - let _ = execute_simple_s3_request( + let started = execute_simple_s3_request( &self.credentials, s3_simple_request::S3RequestOptions { path: &self.path, @@ -1007,10 +1007,21 @@ impl MultiPartUpload { }, s3_simple_request::S3Callback::Upload(Self::single_send_upload_response), self.as_ctx_ptr(), - ); // TODO: properly propagate exception upwards + ); + if started.is_err() { + // A failed settle leaves an exception pending on the VM; report + // it so it cannot ride the tick into unrelated JS (the error + // label erases Thrown, so probe the VM instead). + self.global_this + .report_active_exception_as_unhandled(bun_jsc::JsError::Thrown); + } } else { // we need to split - let _ = self.process_multi_part(part_size); // TODO: properly propagate exception upwards + if self.process_multi_part(part_size).is_err() { + // See the single-request arm above. + self.global_this + .report_active_exception_as_unhandled(bun_jsc::JsError::Thrown); + } } } diff --git a/test/js/bun/io/bun-write.test.js b/test/js/bun/io/bun-write.test.js index f02bdf4f8e17..2851ce420394 100644 --- a/test/js/bun/io/bun-write.test.js +++ b/test/js/bun/io/bun-write.test.js @@ -943,4 +943,63 @@ int posix_fadvise(int fd, off_t offset, off_t len, int advice) { expect(f.name).toBe(filePath); }); + + // The native write completions (copy_file, write_file, and the locked-body + // wait task) settle their promises from the event loop and must never leave + // an exception riding the tick. Pin both paths with a hostile + // `Object.prototype.then` accessor installed, mirroring the + // prototype-pollution state fuzzer processes run in. + it("write completions settle under Object.prototype.then pollution", async () => { + const fixture = ` + const fs = require("fs"); + const dir = fs.mkdtempSync(require("os").tmpdir() + "/bun-write-pollution-"); + const server = Bun.serve({ + port: 0, + async fetch() { + return new Response(new ReadableStream({ + async pull(c) { + c.enqueue(new TextEncoder().encode("hello ")); + await Bun.sleep(10); + c.enqueue(new TextEncoder().encode("world")); + c.close(); + }, + })); + }, + }); + const resp = await fetch("http://localhost:" + server.port + "/"); + fs.writeFileSync(dir + "/src.txt", "copy me"); + + Object.defineProperty(Object.prototype, "then", { + configurable: true, + get() { + throw new Error("boom"); + }, + }); + // Body still streaming: Bun.write waits on the locked body, then + // resolves its promise with the inner write's promise. + const wroteStream = await Bun.write(dir + "/a.txt", resp); + // copy_file completion resolves with the byte count. + const wroteCopy = await Bun.write(Bun.file(dir + "/dst.txt"), Bun.file(dir + "/src.txt")); + delete Object.prototype.then; + + const results = [ + wroteStream, + fs.readFileSync(dir + "/a.txt", "utf8"), + wroteCopy, + fs.readFileSync(dir + "/dst.txt", "utf8"), + ]; + server.stop(true); + console.log(JSON.stringify(results)); + `; + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", fixture], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(JSON.parse(stdout)).toEqual([11, "hello world", 7, "copy me"]); + expect(exitCode).toBe(0); + }); }); diff --git a/test/js/bun/spawn/spawn.test.ts b/test/js/bun/spawn/spawn.test.ts index 5b6da5beda1c..400a65f81c79 100644 --- a/test/js/bun/spawn/spawn.test.ts +++ b/test/js/bun/spawn/spawn.test.ts @@ -1536,3 +1536,40 @@ describe("uid/gid", () => { expect(thrown?.code).toBe("EPERM"); }); }); + +// The native exit completion settles the cached `exited` promise and then +// immediately runs more JS (onExit callback, IPC teardown) in the same task, +// so a failed settle must never leave its exception pending on the VM. The +// settle values are primitives, so a hostile `Object.prototype.then` accessor +// (prototype pollution persists across scripts in fuzzer processes) must not +// be consulted and both the exit-code and signal paths must stay healthy. +it.if(isPosix)("exited settles under Object.prototype.then pollution", async () => { + await using proc = spawn({ + cmd: [ + bunExe(), + "-e", + ` + Object.defineProperty(Object.prototype, "then", { + configurable: true, + get() { + throw new Error("boom"); + }, + }); + const exited7 = Bun.spawn({ cmd: ["sh", "-c", "exit 7"] }).exited; + const killed = Bun.spawn({ cmd: ["sleep", "1000"] }); + const exitedKilled = killed.exited; + killed.kill("SIGKILL"); + const codes = [await exited7, await exitedKilled]; + delete Object.prototype.then; + console.log(JSON.stringify(codes)); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(JSON.parse(stdout)).toEqual([7, 137]); + expect(exitCode).toBe(0); +}); From 15cf7ba3aa4fb59cece62fa477b58917d89c6cdf Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 6 Aug 2026 21:10:06 +0000 Subject: [PATCH 2/4] test: use harness tempDir in the write-pollution fixture, pin Windows copyfile count The Windows copyfile completion resolves 0 when the size is not known up front (uv_fs_copyfile reports no byte count), so the expectation is platform-aware. The fixture directory now comes from the harness so it is removed on every exit path. --- test/js/bun/io/bun-write.test.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/js/bun/io/bun-write.test.js b/test/js/bun/io/bun-write.test.js index 2851ce420394..d07ae95760bc 100644 --- a/test/js/bun/io/bun-write.test.js +++ b/test/js/bun/io/bun-write.test.js @@ -950,9 +950,10 @@ int posix_fadvise(int fd, off_t offset, off_t len, int advice) { // `Object.prototype.then` accessor installed, mirroring the // prototype-pollution state fuzzer processes run in. it("write completions settle under Object.prototype.then pollution", async () => { + using dir = tempDir("bun-write-pollution", {}); const fixture = ` const fs = require("fs"); - const dir = fs.mkdtempSync(require("os").tmpdir() + "/bun-write-pollution-"); + const dir = ${JSON.stringify(String(dir))}; const server = Bun.serve({ port: 0, async fetch() { @@ -999,7 +1000,9 @@ int posix_fadvise(int fd, off_t offset, off_t len, int advice) { }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect(stderr).toBe(""); - expect(JSON.parse(stdout)).toEqual([11, "hello world", 7, "copy me"]); + // uv_fs_copyfile does not report a byte count, so the Windows copyfile + // completion resolves 0 when the size is not known up front. + expect(JSON.parse(stdout)).toEqual([11, "hello world", isWindows ? 0 : 7, "copy me"]); expect(exitCode).toBe(0); }); }); From 32a2fb7e3af2a7c080cb79f8cec2b049457f0345 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 6 Aug 2026 21:26:08 +0000 Subject: [PATCH 3/4] test: pin only the type of the copyfile byte count The resolved count on Windows depends on the backend: uv_fs_copyfile reports none (so the promise resolves 0) while the fallback loop used under BUN_FEATURE_FLAG_DISABLE_UV_FS_COPYFILE reports the real count, and this file reruns itself with that flag. The destination content check is the meaningful assertion. --- test/js/bun/io/bun-write.test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/js/bun/io/bun-write.test.js b/test/js/bun/io/bun-write.test.js index d07ae95760bc..85aeb82c9767 100644 --- a/test/js/bun/io/bun-write.test.js +++ b/test/js/bun/io/bun-write.test.js @@ -979,14 +979,16 @@ int posix_fadvise(int fd, off_t offset, off_t len, int advice) { // Body still streaming: Bun.write waits on the locked body, then // resolves its promise with the inner write's promise. const wroteStream = await Bun.write(dir + "/a.txt", resp); - // copy_file completion resolves with the byte count. + // copy_file completion. The resolved count is backend-dependent on + // Windows (uv_fs_copyfile reports none, the fallback loop the real + // count), so pin only its type; the content check below is the proof. const wroteCopy = await Bun.write(Bun.file(dir + "/dst.txt"), Bun.file(dir + "/src.txt")); delete Object.prototype.then; const results = [ wroteStream, fs.readFileSync(dir + "/a.txt", "utf8"), - wroteCopy, + typeof wroteCopy, fs.readFileSync(dir + "/dst.txt", "utf8"), ]; server.stop(true); @@ -1000,9 +1002,7 @@ int posix_fadvise(int fd, off_t offset, off_t len, int advice) { }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect(stderr).toBe(""); - // uv_fs_copyfile does not report a byte count, so the Windows copyfile - // completion resolves 0 when the size is not known up front. - expect(JSON.parse(stdout)).toEqual([11, "hello world", isWindows ? 0 : 7, "copy me"]); + expect(JSON.parse(stdout)).toEqual([11, "hello world", "number", "copy me"]); expect(exitCode).toBe(0); }); }); From 25c22c6153c983ee9abfc4abbef67d35189d4d27 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 6 Aug 2026 21:52:17 +0000 Subject: [PATCH 4/4] blob: report failed settles in the POSIX write/copy task completions The POSIX halves of the Bun.write completions (WriteFilePromise::run, CopyFile::then/reject) return into the task dispatch channel, whose error type is the terminated sentinel; a thrown exception from a failed settle was mislabeled as termination, stopped the tick drain early, and stayed pending. Consult the VM via report_error_or_terminate instead: report a pending non-termination exception and return Ok, keep real termination as the Err that unwinds the tick loop. WriteFilePromise::run is the completion callback on both platforms, so the Windows write path now reports there too instead of inside run_from_js_thread. --- src/runtime/webcore/blob/copy_file.rs | 24 +++++++++++--- src/runtime/webcore/blob/write_file.rs | 43 ++++++++++++++------------ test/js/bun/io/bun-write.test.js | 3 +- 3 files changed, 43 insertions(+), 27 deletions(-) diff --git a/src/runtime/webcore/blob/copy_file.rs b/src/runtime/webcore/blob/copy_file.rs index 4e2c40e6b7e2..a7a32d11405f 100644 --- a/src/runtime/webcore/blob/copy_file.rs +++ b/src/runtime/webcore/blob/copy_file.rs @@ -137,7 +137,14 @@ impl<'a> CopyFile<'a> { if let Some(store) = self.store.take() { drop(store); // deref() } - promise.reject(global_this, Ok(instance)) + if promise.reject(global_this, Ok(instance)).is_err() { + // The settle error label erases Thrown, so probe the VM: report a + // pending non-termination exception instead of letting the task + // channel treat it as termination, and keep real termination as + // the `Err` that unwinds the tick loop. + return jsc::task::report_error_or_terminate(global_this, jsc::JsError::Thrown); + } + Ok(()) } pub(crate) fn then(&mut self, promise: &mut JSPromise) -> Result<(), jsc::JsTerminated> { @@ -147,10 +154,17 @@ impl<'a> CopyFile<'a> { return self.reject(promise); } - promise.resolve( - self.global_this, - JSValue::js_number_from_uint64(self.read_len as u64), - ) + if promise + .resolve( + self.global_this, + JSValue::js_number_from_uint64(self.read_len as u64), + ) + .is_err() + { + // See `reject` above. + return jsc::task::report_error_or_terminate(self.global_this, jsc::JsError::Thrown); + } + Ok(()) } #[cfg(not(windows))] diff --git a/src/runtime/webcore/blob/write_file.rs b/src/runtime/webcore/blob/write_file.rs index 73579a43ede1..b4ea1ccec8a2 100644 --- a/src/runtime/webcore/blob/write_file.rs +++ b/src/runtime/webcore/blob/write_file.rs @@ -927,7 +927,8 @@ mod windows_impl { ) } { WriteFileWindowsError::WriteFileWindowsDeinitialized => {} - // `run_from_js_thread` already reported the failed settle. + // The completion callback reported any failed settle; + // JSTerminated is real termination, left pending. WriteFileWindowsError::JSTerminated => {} } return; @@ -941,7 +942,8 @@ mod windows_impl { if let Err(e) = unsafe { Self::do_write_loop(this, (*this).loop_()) } { match e { WriteFileWindowsError::WriteFileWindowsDeinitialized => {} - // `run_from_js_thread` already reported the failed settle. + // The completion callback reported any failed settle; + // JSTerminated is real termination, left pending. WriteFileWindowsError::JSTerminated => {} } } @@ -991,7 +993,8 @@ mod windows_impl { // SAFETY: caller contract — `this` is live; `throw` consumes it. match unsafe { Self::throw(this, err_) } { WriteFileWindowsError::WriteFileWindowsDeinitialized => {} - // `run_from_js_thread` already reported the failed settle. + // The completion callback reported any failed settle; + // JSTerminated is real termination, left pending. WriteFileWindowsError::JSTerminated => {} } return; @@ -1001,7 +1004,8 @@ mod windows_impl { if let Err(e) = unsafe { Self::open(this) } { match e { WriteFileWindowsError::WriteFileWindowsDeinitialized => {} - // `run_from_js_thread` already reported the failed settle. + // The completion callback reported any failed settle; + // JSTerminated is real termination, left pending. WriteFileWindowsError::JSTerminated => {} } } @@ -1009,7 +1013,7 @@ mod windows_impl { /// `ManagedTask`-shaped trampoline for [`on_mkdirp_complete`]: takes /// `*mut Self` and returns the event-loop `JsResult<()>` (always `Ok`; - /// a failed settle is reported inside `run_from_js_thread`). + /// a failed settle is reported inside the completion callback). fn on_mkdirp_complete_task(this: *mut WriteFileWindows) -> bun_event_loop::JsResult<()> { // SAFETY: `this` is the live Box-allocated `WriteFileWindows` whose // pointer was stashed in `on_mkdirp_complete_concurrent` below; @@ -1063,7 +1067,8 @@ mod windows_impl { ) } { WriteFileWindowsError::WriteFileWindowsDeinitialized => {} - // `run_from_js_thread` already reported the failed settle. + // The completion callback reported any failed settle; + // JSTerminated is real termination, left pending. WriteFileWindowsError::JSTerminated => {} } return; @@ -1075,7 +1080,8 @@ mod windows_impl { if let Err(e) = unsafe { Self::do_write_loop(this, (*this).loop_()) } { match e { WriteFileWindowsError::WriteFileWindowsDeinitialized => {} - // `run_from_js_thread` already reported the failed settle. + // The completion callback reported any failed settle; + // JSTerminated is real termination, left pending. WriteFileWindowsError::JSTerminated => {} } } @@ -1102,19 +1108,12 @@ mod windows_impl { // SAFETY: caller contract — `this` is live; copy out everything we // need before `deinit` frees the allocation. let (cb, cb_ctx) = unsafe { ((*this).on_complete_callback, (*this).on_complete_ctx) }; - // SAFETY: caller contract — `this` is live; the VM-owned event - // loop (and its global) outlives the request. - let global = unsafe { (*(*this).event_loop).global_ref() }; // 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))) { - // A failed settle leaves an exception pending on the VM; - // report it so it cannot ride the tick into unrelated JS - // (the error label erases Thrown, so probe the VM instead). - global.report_active_exception_as_unhandled(jsc::JsError::Thrown); return e.into(); } } else { @@ -1123,8 +1122,6 @@ mod windows_impl { // SAFETY: caller contract — `this` is live; consumed here. unsafe { Self::deinit(this) }; if let Err(e) = cb(cb_ctx, WriteFileResultType::Result(wrote as SizeType)) { - // See the error arm above. - global.report_active_exception_as_unhandled(jsc::JsError::Thrown); return e.into(); } } @@ -1318,22 +1315,28 @@ impl WriteFilePromise { // SAFETY: GC-owned cell (kept alive below); scoped shared access. let value = unsafe { (*promise).to_js() }; value.ensure_still_alive(); - match count { + let settled = 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))?; + (*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))?; + (*promise).resolve(global_this, JSValue::js_number_from_uint64(wrote as u64)) } } + }; + if settled.is_err() { + // The settle error label erases Thrown, so probe the VM: report a + // pending non-termination exception instead of letting the task + // channel treat it as termination, and keep real termination as + // the `Err` that unwinds the tick loop. + return jsc::task::report_error_or_terminate(global_this, jsc::JsError::Thrown); } Ok(()) } diff --git a/test/js/bun/io/bun-write.test.js b/test/js/bun/io/bun-write.test.js index 85aeb82c9767..4d0df4a7e500 100644 --- a/test/js/bun/io/bun-write.test.js +++ b/test/js/bun/io/bun-write.test.js @@ -950,7 +950,7 @@ int posix_fadvise(int fd, off_t offset, off_t len, int advice) { // `Object.prototype.then` accessor installed, mirroring the // prototype-pollution state fuzzer processes run in. it("write completions settle under Object.prototype.then pollution", async () => { - using dir = tempDir("bun-write-pollution", {}); + using dir = tempDir("bun-write-pollution", { "src.txt": "copy me" }); const fixture = ` const fs = require("fs"); const dir = ${JSON.stringify(String(dir))}; @@ -968,7 +968,6 @@ int posix_fadvise(int fd, off_t offset, off_t len, int advice) { }, }); const resp = await fetch("http://localhost:" + server.port + "/"); - fs.writeFileSync(dir + "/src.txt", "copy me"); Object.defineProperty(Object.prototype, "then", { configurable: true,