diff --git a/src/jsc/bindings/ZigGlobalObject.cpp b/src/jsc/bindings/ZigGlobalObject.cpp index a45cf0016dfe..c5a93f71bce0 100644 --- a/src/jsc/bindings/ZigGlobalObject.cpp +++ b/src/jsc/bindings/ZigGlobalObject.cpp @@ -4080,6 +4080,10 @@ GlobalObject::PromiseFunctions GlobalObject::promiseHandlerID(Zig::FFIFunction h return GlobalObject::PromiseFunctions::Bun__HTTPRequestContextDebugH3__onResolve; } else if (handler == Bun__HTTPRequestContextDebugH3__onResolveStream) { return GlobalObject::PromiseFunctions::Bun__HTTPRequestContextDebugH3__onResolveStream; + } else if (handler == Bun__JSBundler__onResolvePluginSetup) { + return GlobalObject::PromiseFunctions::Bun__JSBundler__onResolvePluginSetup; + } else if (handler == Bun__JSBundler__onRejectPluginSetup) { + return GlobalObject::PromiseFunctions::Bun__JSBundler__onRejectPluginSetup; } else { RELEASE_ASSERT_NOT_REACHED(); } diff --git a/src/jsc/bindings/ZigGlobalObject.h b/src/jsc/bindings/ZigGlobalObject.h index ca759a74da9e..1c61279bb4d4 100644 --- a/src/jsc/bindings/ZigGlobalObject.h +++ b/src/jsc/bindings/ZigGlobalObject.h @@ -412,8 +412,10 @@ class GlobalObject : public Bun::GlobalScope { Bun__HTTPRequestContextDebugH3__onRejectStream, Bun__HTTPRequestContextDebugH3__onResolve, Bun__HTTPRequestContextDebugH3__onResolveStream, + Bun__JSBundler__onResolvePluginSetup, + Bun__JSBundler__onRejectPluginSetup, }; - static constexpr size_t promiseFunctionsSize = 42; + static constexpr size_t promiseFunctionsSize = 44; static PromiseFunctions promiseHandlerID(SYSV_ABI EncodedJSValue (*handler)(JSC::JSGlobalObject* arg0, JSC::CallFrame* arg1)); diff --git a/src/jsc/bindings/headers.h b/src/jsc/bindings/headers.h index ab056ebc60bd..3356b5dc1b3d 100644 --- a/src/jsc/bindings/headers.h +++ b/src/jsc/bindings/headers.h @@ -719,6 +719,9 @@ ZIG_DECL JSC::EncodedJSValue Bun__Timer__setImmediate(JSC::JSGlobalObject* globa BUN_DECLARE_HOST_FUNCTION(BunServe__onResolvePlugins); BUN_DECLARE_HOST_FUNCTION(BunServe__onRejectPlugins); +BUN_DECLARE_HOST_FUNCTION(Bun__JSBundler__onResolvePluginSetup); +BUN_DECLARE_HOST_FUNCTION(Bun__JSBundler__onRejectPluginSetup); + #endif #ifdef __cplusplus diff --git a/src/runtime/api/JSBundler.rs b/src/runtime/api/JSBundler.rs index cfbd22382da3..8c8ad2113d74 100644 --- a/src/runtime/api/JSBundler.rs +++ b/src/runtime/api/JSBundler.rs @@ -434,12 +434,38 @@ pub mod js_bundler { } } + /// One resolved step of the plugin `setup()` chain. + enum SetupStep { + /// The updated `onStart()` promise array, threaded into the next step. + Settled(JSValue), + /// A still-pending async `setup()` or last-plugin `onStart()` `Promise.all`. + Pending(JSValue), + } + + /// Where [`Config::from_js`] stopped when a plugin `setup()` suspended; see [`DeferredBuild`]. + pub struct SuspendedPluginSetup { + /// GC-roots the `runSetupFunction` promise until `build` attaches `.then` reactions. + pending: jsc::Strong, + /// `config.plugins`, snapshotted so getters aren't re-invoked on resume. + plugins_array: jsc::Strong, + /// Plugin index to resume the chain from. + next_index: u32, + /// Snapshot of the plugin array length. + length: u32, + plugin_target: jsc::BunPluginTarget, + /// Only `target` is set; the rest is parsed by `finish_from_js` once the chain settles. + config: Config, + did_set_target: bool, + } + impl Config { + /// `Ok(None)` means a plugin `setup()` suspended; `*suspended` records where the chain stopped (#33261). pub fn from_js( global_this: &JSGlobalObject, config: JSValue, plugins: &mut Option<*mut Plugin>, - ) -> JsResult { + suspended: &mut Option, + ) -> JsResult> { // Config implements Drop, so functional-record-update from Default::default() // is rejected by rustc (E0509). Construct default then mutate instead. let mut this = Config::default(); @@ -481,94 +507,139 @@ pub mod js_bundler { // Plugins must be resolved first as they are allowed to mutate the config JSValue if let Some(array) = config.get_array(global_this, "plugins")? { - let length = array.get_length(global_this)?; - let mut iter = array.array_iterator(global_this)?; + let length = array.get_length(global_this)? as u32; + let plugin_target = match this.target { + Target::Bun | Target::BunMacro => jsc::BunPluginTarget::Bun, + Target::Node => jsc::BunPluginTarget::Node, + _ => jsc::BunPluginTarget::Browser, + }; let mut onstart_promise_array = JSValue::UNDEFINED; - let mut i: usize = 0; - while let Some(plugin) = iter.next()? { - if !plugin.is_object() { - return Err(global_this.throw_invalid_arguments(format_args!( - "Expected plugin to be an object" - ))); - } - - if let Some(slice) = plugin.get_optional_slice(global_this, b"name")? { - if slice.slice().is_empty() { - return Err(global_this.throw_invalid_arguments(format_args!( - "Expected plugin to have a non-empty name" - ))); + for index in 0..length { + match Self::run_one_plugin_setup( + global_this, + config, + &mut **plugins, + plugin_target, + array, + index, + length, + onstart_promise_array, + )? { + SetupStep::Settled(value) => onstart_promise_array = value, + SetupStep::Pending(pending) => { + *suspended = Some(SuspendedPluginSetup { + pending: jsc::Strong::create(pending, global_this), + plugins_array: jsc::Strong::create(array, global_this), + next_index: index + 1, + length, + plugin_target, + config: this, + did_set_target, + }); + // Plugin registry ownership moves to the continuation; disarm cleanup. + scopeguard::ScopeGuard::into_inner(plugins); + return Ok(None); } - drop(slice); - } else { - return Err(global_this.throw_invalid_arguments(format_args!( - "Expected plugin to have a name" - ))); } + } + } - let Some(function) = plugin.get_function(global_this, b"setup")? else { - return Err(global_this.throw_invalid_arguments(format_args!( - "Expected plugin to have a setup() function" - ))); - }; + let this = Self::finish_from_js(global_this, config, this, did_set_target)?; + scopeguard::ScopeGuard::into_inner(plugins); + Ok(Some(this)) + } - let bun_plugins: *mut Plugin = match **plugins { - Some(p) => p, - None => { - let p = Plugin::create( - global_this, - match this.target { - Target::Bun | Target::BunMacro => jsc::BunPluginTarget::Bun, - Target::Node => jsc::BunPluginTarget::Node, - _ => jsc::BunPluginTarget::Browser, - }, - ); - **plugins = Some(p); - p - } - }; + /// Run plugin `index`'s `setup()`; one at a time because the builtin stashes the prior step's array. + #[allow(clippy::too_many_arguments)] + fn run_one_plugin_setup( + global_this: &JSGlobalObject, + config: JSValue, + plugins: &mut Option<*mut Plugin>, + plugin_target: jsc::BunPluginTarget, + array: JSValue, + index: u32, + length: u32, + onstart_promise_array: JSValue, + ) -> JsResult { + let plugin = array.get_index(global_this, index)?; + if !plugin.is_object() { + return Err(global_this + .throw_invalid_arguments(format_args!("Expected plugin to be an object"))); + } - let is_last = i == (length as usize).saturating_sub(1); - // SAFETY: bun_plugins is a valid pointer created/stored above - let mut plugin_result = unsafe { - (*bun_plugins).add_plugin( - function, - config, - onstart_promise_array, - is_last, - false, - )? - }; + if let Some(slice) = plugin.get_optional_slice(global_this, b"name")? { + if slice.slice().is_empty() { + return Err(global_this.throw_invalid_arguments(format_args!( + "Expected plugin to have a non-empty name" + ))); + } + drop(slice); + } else { + return Err(global_this + .throw_invalid_arguments(format_args!("Expected plugin to have a name"))); + } - if !plugin_result.is_empty_or_undefined_or_null() { - if let Some(promise) = plugin_result.as_any_promise() { - promise.set_handled(global_this.vm()); - // SAFETY: bun_vm() returns the live process VirtualMachine pointer. - global_this.bun_vm().as_mut().wait_for_promise(promise); - match promise - .unwrap(global_this.vm(), jsc::PromiseUnwrapMode::MarkHandled) - { - jsc::PromiseResult::Pending => unreachable!(), - jsc::PromiseResult::Fulfilled(val) => { - plugin_result = val; - } - jsc::PromiseResult::Rejected(err) => { - return Err(global_this.throw_value(err)); - } - } - } - } + let Some(function) = plugin.get_function(global_this, b"setup")? else { + return Err(global_this.throw_invalid_arguments(format_args!( + "Expected plugin to have a setup() function" + ))); + }; - if let Some(err) = plugin_result.to_error() { - return Err(global_this.throw_value(err)); - } else if global_this.has_exception() { - return Err(JsError::Thrown); - } + let bun_plugins: *mut Plugin = match *plugins { + Some(p) => p, + None => { + let p = Plugin::create(global_this, plugin_target); + *plugins = Some(p); + p + } + }; + + let is_last = index + 1 == length; + // SAFETY: bun_plugins is a valid pointer created/stored above + let mut plugin_result = unsafe { + (*bun_plugins).add_plugin( + function, + config, + onstart_promise_array, + is_last, + false, + )? + }; - onstart_promise_array = plugin_result; - i += 1; + if !plugin_result.is_empty_or_undefined_or_null() { + if let Some(promise) = plugin_result.as_any_promise() { + promise.set_handled(global_this.vm()); + if matches!(promise.status(), jsc::js_promise::Status::Pending) { + return Ok(SetupStep::Pending(plugin_result)); + } + match promise.unwrap(global_this.vm(), jsc::PromiseUnwrapMode::MarkHandled) { + jsc::PromiseResult::Pending => unreachable!(), + jsc::PromiseResult::Fulfilled(val) => { + plugin_result = val; + } + jsc::PromiseResult::Rejected(err) => { + return Err(global_this.throw_value(err)); + } + } } } + if let Some(err) = plugin_result.to_error() { + return Err(global_this.throw_value(err)); + } else if global_this.has_exception() { + return Err(JsError::Thrown); + } + + Ok(SetupStep::Settled(plugin_result)) + } + + /// The post-setup tail of `from_js`; plugins may mutate the config, so nothing here can be read earlier. + fn finish_from_js( + global_this: &JSGlobalObject, + config: JSValue, + mut this: Config, + did_set_target: bool, + ) -> JsResult { if let Some(macros_flag) = config.get_boolean_loose(global_this, "macros")? { this.no_macros = !macros_flag; } @@ -1278,7 +1349,6 @@ pub mod js_bundler { } } - scopeguard::ScopeGuard::into_inner(plugins); Ok(this) } } @@ -1337,7 +1407,34 @@ pub mod js_bundler { } let mut plugins: Option<*mut Plugin> = None; - let config = Config::from_js(global_this, arguments[0], &mut plugins)?; + let mut suspended: Option = None; + let Some(config) = + Config::from_js(global_this, arguments[0], &mut plugins, &mut suspended)? + else { + let suspended = suspended.expect("from_js returned None without a suspension"); + let result = jsc::JSPromiseStrong::init(global_this); + let return_value = result.value(); + // `suspended.pending` stays rooted through this block and drops after `.then` attaches. + let pending = suspended.pending.get(); + let deferred = bun_core::heap::into_raw(Box::new(DeferredBuild { + config_js: jsc::Strong::create(arguments[0], global_this), + plugins_array: suspended.plugins_array, + next_index: suspended.next_index, + length: suspended.length, + plugin_target: suspended.plugin_target, + plugin: plugins, + partial: Some(suspended.config), + did_set_target: suspended.did_set_target, + result, + })); + pending.then( + global_this, + deferred, + __jsc_host_on_deferred_build_resolve, + __jsc_host_on_deferred_build_reject, + ); + return Ok(return_value); + }; let event_loop = vm.event_loop(); @@ -1370,6 +1467,141 @@ pub mod js_bundler { build(global_this, callframe.arguments()) } + /// Leaked via `heap::into_raw`; exactly one `.then` reaction reclaims it (or re-leaks on another suspension). + struct DeferredBuild { + /// `Bun.build`'s config argument; re-read by `finish_from_js` after the chain settles. + config_js: jsc::Strong, + /// See [`SuspendedPluginSetup::plugins_array`]. + plugins_array: jsc::Strong, + next_index: u32, + length: u32, + plugin_target: jsc::BunPluginTarget, + /// Owned here until moved to the completion task or destroyed on failure. + plugin: Option<*mut Plugin>, + /// The pre-plugin config; `None` once it moves to the completion task. + partial: Option, + did_set_target: bool, + /// `Bun.build`'s result promise; moves to the completion task or is rejected here. + result: jsc::JSPromiseStrong, + } + + impl DeferredBuild { + /// Continue from `next_index`; returns `Some(pending)` on another suspension. + fn resume( + &mut self, + global_this: &JSGlobalObject, + mut onstart_promise_array: JSValue, + ) -> JsResult> { + // Preserve the sync chain's reject-on-Error for a `.then`-settled step. + if let Some(err) = onstart_promise_array.to_error() { + return Err(global_this.throw_value(err)); + } + + let array = self.plugins_array.get(); + while self.next_index < self.length { + let index = self.next_index; + self.next_index += 1; + match Config::run_one_plugin_setup( + global_this, + self.config_js.get(), + &mut self.plugin, + self.plugin_target, + array, + index, + self.length, + onstart_promise_array, + )? { + SetupStep::Settled(value) => onstart_promise_array = value, + SetupStep::Pending(pending) => return Ok(Some(pending)), + } + } + + let partial = self + .partial + .take() + .expect("DeferredBuild resumed after completion"); + let config = Config::finish_from_js( + global_this, + self.config_js.get(), + partial, + self.did_set_target, + )?; + + let event_loop = global_this.bun_vm().event_loop(); + let completion = + crate::api::js_bundle_completion_task::create_and_schedule_completion_task( + config, + self.plugin.take().and_then(core::ptr::NonNull::new), + global_this, + event_loop, + ) + .map_err(|_| JsError::OutOfMemory)?; + // SAFETY: `completion` is the freshly-boxed allocation returned + // above; sole owner on the JS thread until the enqueued task runs. + unsafe { + (*completion).promise = self.result.take(); + } + Ok(None) + } + + /// Reject the result promise with the pending exception and free the plugin registry. + fn fail(&mut self, global_this: &JSGlobalObject, err: JsError) { + if let Some(plugin) = self.plugin.take() { + Plugin::destroy(plugin); + } + if matches!(err, JsError::Terminated) { + return; + } + let exception = global_this.take_exception(err); + let _ = self.result.reject(global_this, Ok(exception)); + } + } + + /// `.then` resolve reaction; arg[0] is the settled `onStart()` promise array. + #[bun_jsc::host_fn(export = "Bun__JSBundler__onResolvePluginSetup")] + fn on_deferred_build_resolve( + global_this: &JSGlobalObject, + callframe: &CallFrame, + ) -> JsResult { + let [onstart_promise_array, ctx] = callframe.arguments_as_array::<2>(); + let deferred: *mut DeferredBuild = ctx.as_promise_ptr::(); + // SAFETY: `deferred` was leaked via `heap::into_raw` by `build` (or a + // previous re-suspension) and exactly one of the two `.then` + // reactions runs; reclaiming it here is the unique owner transition. + let mut this: Box = unsafe { bun_core::heap::take(deferred) }; + match this.resume(global_this, onstart_promise_array) { + Ok(Some(pending)) => { + let deferred = bun_core::heap::into_raw(this); + pending.then( + global_this, + deferred, + __jsc_host_on_deferred_build_resolve, + __jsc_host_on_deferred_build_reject, + ); + } + Ok(None) => {} + Err(err) => this.fail(global_this, err), + } + Ok(JSValue::UNDEFINED) + } + + /// `.then` reject reaction for a suspended plugin `setup()` chain. + #[bun_jsc::host_fn(export = "Bun__JSBundler__onRejectPluginSetup")] + fn on_deferred_build_reject( + global_this: &JSGlobalObject, + callframe: &CallFrame, + ) -> JsResult { + let [error, ctx] = callframe.arguments_as_array::<2>(); + let deferred: *mut DeferredBuild = ctx.as_promise_ptr::(); + // SAFETY: see `on_deferred_build_resolve`. + let mut this: Box = unsafe { bun_core::heap::take(deferred) }; + if let Some(plugin) = this.plugin.take() { + Plugin::destroy(plugin); + } + let _ = this.result.reject(global_this, Ok(error)); + Ok(JSValue::UNDEFINED) + } + // NOTE: `Resolve`/`Load`/`MiniImportRecord`/etc. are owned by // `bun_bundler::bundle_v2::api::JSBundler` so that `BundleV2` can operate // on them directly (`on_resolve_async`/`on_load_async`). `dispatch()` and diff --git a/test/bundler/bun-build-plugin-async-setup.test.ts b/test/bundler/bun-build-plugin-async-setup.test.ts new file mode 100644 index 000000000000..585fee5fe57e --- /dev/null +++ b/test/bundler/bun-build-plugin-async-setup.test.ts @@ -0,0 +1,219 @@ +import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; + +// https://github.com/oven-sh/bun/issues/33261 +// `Bun.build` must not block the event loop on a pending plugin setup() +// promise; it resumes config parsing in that promise's `.then` continuation. +describe.concurrent("Bun.build with an async plugin setup()", () => { + const entry = `export const hello = "world";\n`; + + test("returns before a pending setup() promise settles", async () => { + using dir = tempDir("bun-build-async-setup", { + "entry.js": entry, + "build-fixture.ts": /* ts */ ` + let release!: () => void; + const gate = new Promise(resolve => (release = resolve)); + const order: string[] = []; + + const built = Bun.build({ + entrypoints: ["./entry.js"], + plugins: [ + { + name: "gated", + async setup(build) { + order.push("setup:start"); + await gate; + order.push("setup:resume"); + // A filter registered after the await must still be + // installed: they only take effect once setup() settles. + build.onLoad({ filter: /entry\\.js$/ }, () => ({ + contents: "export const hello = 'patched';", + loader: "js", + })); + }, + }, + ], + }); + + // Unreachable when Bun.build spins the event loop: release() is the + // only thing that can settle the promise it would be waiting on. + order.push("returned"); + release(); + + const result = await built; + order.push("built"); + if (!result.success) throw new AggregateError(result.logs, "build failed"); + const text = await result.outputs[0].text(); + if (!text.includes("patched")) { + throw new Error("onLoad registered after an await was ignored:\\n" + text); + } + console.log(JSON.stringify(order)); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "build-fixture.ts"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + // Only reached when Bun.build blocks; the non-blocking path exits in + // well under a second. + timeout: 15_000, + killSignal: "SIGKILL", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(exitCode === 0 ? JSON.parse(stdout.trim()) : { exitCode, stdout, stderr }).toEqual([ + "setup:start", + "returned", + "setup:resume", + "built", + ]); + }, 30_000); + + test("still waits for a setup() that suspends on I/O", async () => { + using dir = tempDir("bun-build-async-setup-io", { "entry.js": entry }); + let setupResumed = false; + const result = await Bun.build({ + entrypoints: [`${dir}/entry.js`], + plugins: [ + { + name: "sleepy", + async setup() { + await Bun.sleep(10); + setupResumed = true; + }, + }, + ], + }); + expect(setupResumed).toBe(true); + expect(result.success).toBe(true); + }); + + test("still waits for pending onStart() promises before loading any file", async () => { + using dir = tempDir("bun-build-async-setup-onstart", { "entry.js": entry }); + const order: string[] = []; + const result = await Bun.build({ + entrypoints: [`${dir}/entry.js`], + plugins: [ + { + name: "starter", + setup(build) { + build.onStart(async () => { + await Bun.sleep(10); + order.push("onStart"); + }); + build.onLoad({ filter: /entry\.js$/ }, () => { + order.push("onLoad"); + return { contents: entry, loader: "js" }; + }); + }, + }, + ], + }); + expect(result.success).toBe(true); + expect(order).toEqual(["onStart", "onLoad"]); + }); + + test("runs the remaining plugins after an earlier async setup() settles", async () => { + using dir = tempDir("bun-build-async-setup-chain", { "entry.js": entry }); + const order: string[] = []; + const result = await Bun.build({ + entrypoints: [`${dir}/entry.js`], + plugins: [ + { + name: "first", + async setup() { + order.push("first:start"); + await Bun.sleep(10); + order.push("first:done"); + }, + }, + { + name: "second", + setup() { + order.push("second"); + }, + }, + ], + }); + expect(result.success).toBe(true); + expect(order).toEqual(["first:start", "first:done", "second"]); + }); + + test("config mutated after an await in setup() is respected", async () => { + using dir = tempDir("bun-build-async-setup-mutate", { + "entry.js": `export const flag = BUILD_FLAG;\n`, + }); + const result = await Bun.build({ + entrypoints: [`${dir}/entry.js`], + plugins: [ + { + name: "definer", + async setup(build) { + await Bun.sleep(10); + build.config.define = { BUILD_FLAG: JSON.stringify("after-await") }; + }, + }, + ], + }); + expect(result.success).toBe(true); + expect(await result.outputs[0].text()).toContain("after-await"); + }); + + test("lets a caller escape a never-settling setup() with Promise.race", async () => { + using dir = tempDir("bun-build-async-setup-race", { + "entry.js": entry, + "race-fixture.ts": /* ts */ ` + const cfg = { + entrypoints: ["./entry.js"], + plugins: [ + { + name: "never-settles", + async setup() { + await new Promise(() => {}); + }, + }, + ], + }; + const buildPromise = Bun.build(cfg); + const winner = await Promise.race([ + buildPromise.then(() => "build"), + Bun.sleep(200).then(() => "timeout"), + ]); + console.log(JSON.stringify({ winner })); + process.exit(winner === "timeout" ? 0 : 1); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "race-fixture.ts"], + env: bunEnv, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + timeout: 15_000, + killSignal: "SIGKILL", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(exitCode === 0 ? JSON.parse(stdout.trim()) : { exitCode, stdout, stderr }).toEqual({ winner: "timeout" }); + }, 30_000); + + test("rejects the build promise when an async setup() rejects", async () => { + using dir = tempDir("bun-build-async-setup-reject", { "entry.js": entry }); + await expect( + Bun.build({ + entrypoints: [`${dir}/entry.js`], + plugins: [ + { + name: "explosive", + async setup() { + await Bun.sleep(10); + throw new Error("setup exploded"); + }, + }, + ], + }), + ).rejects.toThrow("setup exploded"); + }); +});