Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions src/runtime/api/JSBundler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1804,6 +1804,51 @@ pub mod js_bundler {
}
}

/// Owner of a `JSBundlerPlugin` from [`PluginJscExt::create`]. The handle
/// is a `protect()`ed JSCell, not a heap allocation: a `Box<Plugin>` around
/// it frees nothing (`Plugin` is a ZST opaque), so the only thing that
/// releases the cell is this type's `Drop` calling [`PluginJscExt::destroy`].
/// JS thread only.
pub struct OwnedPlugin(core::ptr::NonNull<Plugin>);

impl OwnedPlugin {
pub fn create(global: &JSGlobalObject, target: jsc::BunPluginTarget) -> Self {
Self(
core::ptr::NonNull::new(Plugin::create(global, target))
.expect("JSBundlerPlugin__create returns a non-null cell"),
)
}

/// The handle itself, for holders that only borrow the plugin (the
/// bundle thread, routes and the dev server sharing a server's plugins).
/// Valid until `self` drops.
#[inline]
pub fn as_non_null(&self) -> core::ptr::NonNull<Plugin> {
self.0
}
}

impl core::ops::Deref for OwnedPlugin {
type Target = Plugin;
#[inline]
fn deref(&self) -> &Plugin {
Plugin::opaque_ref(self.0.as_ptr())
}
}

impl core::ops::DerefMut for OwnedPlugin {
#[inline]
fn deref_mut(&mut self) -> &mut Plugin {
Plugin::opaque_mut(self.0.as_ptr())
}
}

impl Drop for OwnedPlugin {
fn drop(&mut self) {
Plugin::destroy(self.0.as_ptr());
}
}

/// Convert a JS exception value into a `logger.Msg`. If the conversion itself
/// throws (e.g. `Symbol.toPrimitive` on the thrown object throws), clear that
/// secondary exception and return a generic fallback message so
Expand Down Expand Up @@ -1880,6 +1925,7 @@ pub mod js_bundler {

pub use js_bundler as JSBundler;
pub use js_bundler::Config;
pub(crate) use js_bundler::OwnedPlugin;
/// `jsc.API.JSBundler.Plugin` — re-exported for `crate::bake` (`SplitBundlerOptions.plugin`).
pub use js_bundler::Plugin;
pub(crate) use js_bundler::PluginJscExt;
Expand Down
36 changes: 26 additions & 10 deletions src/runtime/bake/DevServer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,8 @@ pub enum Magic {

#[derive(Copy, Clone, Eq, PartialEq)]
pub enum PluginState {
/// Should ask server for plugins. Once plugins are loaded, the plugin
/// pointer is written into `server_transpiler.options.plugin`
/// Should ask server for plugins (unless the app brought its own). Once
/// plugins are loaded, the cell is stored in `bundler_options.plugin`.
Unknown,
// These two states mean that `server.getOrLoadPlugins()` was called.
Pending,
Expand Down Expand Up @@ -528,7 +528,6 @@ pub(crate) fn init(options: Options) -> JsResult<Box<DevServer>> {
w!(generation, 0);
w!(graph_safety_lock, ThreadLock::init_unlocked());
w!(framework, options.framework);
w!(bundler_options, options.bundler_options);
w!(emit_incremental_visualizer_events, 0);
w!(emit_memory_visualizer_events, 0);
w!(
Expand Down Expand Up @@ -650,8 +649,11 @@ pub(crate) fn init(options: Options) -> JsResult<Box<DevServer>> {
//
// SAFETY: `init_transpiler` writes the slot via `MaybeUninit::write` (see
// `bake_body.rs`), so the previous (uninitialized) bytes are never dropped.
// `framework`/`log`/`bundler_options` were written above; reborrowing each
// individually via `addr_of_mut!` is sound because no `&mut DevServer` exists.
// `framework`/`log` were written above; reborrowing each individually via
// `addr_of_mut!` is sound because no `&mut DevServer` exists.
// `bundler_options` stays in `options` until the transpilers exist: it owns
// the app's plugin cell, and an `Err` before `assume_init()` drops nothing
// that was already written into the box.
// Note: `Transpiler<'static>` erases the arena lifetime — `options.arena`
// is the `UserOptions.arena` which is moved into / outlives the `DevServer`
// box. Widen `'a → 'static` here once.
Expand All @@ -666,7 +668,7 @@ pub(crate) fn init(options: Options) -> JsResult<Box<DevServer>> {
unsafe {
let framework = &mut *addr_of_mut!((*p).framework);
let log = &mut *addr_of_mut!((*p).log);
let bundler_options = &mut *addr_of_mut!((*p).bundler_options);
let bundler_options = &options.bundler_options;

match framework.init_transpiler(
arena,
Expand Down Expand Up @@ -719,6 +721,7 @@ pub(crate) fn init(options: Options) -> JsResult<Box<DevServer>> {
}

w!(bundler_framework_views, bundler_framework_views);
w!(bundler_options, options.bundler_options);
}

// ── every field is now written ───────────────────────────────────────────
Expand Down Expand Up @@ -2010,8 +2013,11 @@ fn ensure_route_is_bundled<Ctx: EnsureRouteCtx>(
}
crate::server::GetOrStartLoadResult::Ready(ready) => {
dev.plugin_state = PluginState::Loaded;
dev.bundler_options.plugin =
ready.map(::core::ptr::NonNull::from);
dev.bundler_options.plugin = ready.map(|plugin| {
bake::DevServerPlugin::Borrowed(
::core::ptr::NonNull::from(plugin),
)
});
}
}
}
Expand Down Expand Up @@ -3217,7 +3223,11 @@ impl DevServer {
ssr_transpiler: unsafe {
::core::ptr::NonNull::from((*self_ptr).ssr_transpiler.assume_init_mut())
},
plugins: self.bundler_options.plugin,
plugins: self
.bundler_options
.plugin
.as_ref()
.map(bake::DevServerPlugin::as_non_null),
}),
// SAFETY: see `heap_ptr` note above.
unsafe { &*heap_ptr },
Expand Down Expand Up @@ -6128,7 +6138,13 @@ impl DevServer {
&mut self,
plugins: Option<*mut crate::api::js_bundler::Plugin>,
) -> crate::Result<()> {
self.bundler_options.plugin = plugins.and_then(::core::ptr::NonNull::new);
// Only reached when the app declared no plugins of its own
// (`ensure_route_is_bundled` asks the server for plugins just then),
// so this never replaces an `Owned` cell.
debug_assert!(self.bundler_options.plugin.is_none());
self.bundler_options.plugin = plugins
.and_then(::core::ptr::NonNull::new)
.map(bake::DevServerPlugin::Borrowed);
self.plugin_state = PluginState::Loaded;
self.start_next_bundle_if_present();
Ok(())
Expand Down
52 changes: 12 additions & 40 deletions src/runtime/bake/bake_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#![allow(unexpected_cfgs)] // `bun_codegen_embed` is set via RUSTFLAGS (scripts/build/rust.ts) for release/CI builds.

use bun_alloc::ArenaVecExt as _;
use core::ptr::NonNull;

use bun_alloc::Arena; // = bumpalo::Bump
use bun_collections::ArrayHashMap;
Expand All @@ -15,10 +14,7 @@ use bun_jsc::{JSGlobalObject, JSValue, JsError, JsResult, ZigStringSlice};
use bun_options_types::schema as bun_schema;
use bun_paths::{self as paths, PathBuffer};

// `jsc.API.JSBundler.Plugin` — opaque FFI handle for the C++ JSBundlerPlugin.
// Re-exported from `crate::api::js_bundler` so `SplitBundlerOptions.plugin`
// shares the same type the bundler pipeline uses.
pub(crate) use crate::api::js_bundler::Plugin;
use crate::api::js_bundler::OwnedPlugin;
use crate::api::js_bundler::js_bundler::PluginJscExt as _;

// Note: parent `mod.rs` already declares `dev_server` / `framework_router`
Expand Down Expand Up @@ -145,20 +141,6 @@ pub struct UserOptions {
pub(crate) bundler_options: SplitBundlerOptions,
}

impl Drop for UserOptions {
fn drop(&mut self) {
// arena: dropped by Bump's Drop
// allocations: dropped by StringRefList's Drop
if let Some(p) = self.bundler_options.plugin {
// `p` is the FFI handle returned by `Plugin::create` in
// `parse_plugin_array`; `PluginJscExt::destroy` is its paired
// (safe) destructor — it null-checks via `opaque_ref` and
// unprotect()s the JSCell / tombstones the C++ object.
Plugin::destroy(p.as_ptr());
}
}
}

impl UserOptions {
/// Currently, this function must run at the top of the event loop.
pub fn from_js(config: JSValue, global: &JSGlobalObject) -> JsResult<UserOptions> {
Expand Down Expand Up @@ -296,7 +278,11 @@ impl StringRefList {

#[derive(Default)]
pub struct SplitBundlerOptions {
pub plugin: Option<NonNull<Plugin>>,
/// One cell holding both `framework.plugins` and `app.plugins`. Released
/// when this drops (`bun build --app`, or `Bun.serve` rejecting its
/// options) unless `NewServer::init` moves it into the `DevServer`, which
/// then owns it (`bake::DevServerPlugin::Owned`).
pub plugin: Option<OwnedPlugin>,
pub client: BuildConfigSubset,
pub server: BuildConfigSubset,
pub ssr: BuildConfigSubset,
Expand All @@ -312,18 +298,11 @@ impl SplitBundlerOptions {
plugin_array: JSValue,
global: &JSGlobalObject,
) -> JsResult<()> {
// Create the Plugin and assign it to `opts.plugin` BEFORE iterating,
// Create the Plugin and assign it to `self.plugin` BEFORE iterating,
// so `plugins: []` still leaves `self.plugin = Some(_)`.
let plugin: NonNull<Plugin> = match self.plugin {
Some(p) => p,
None => {
let p = Plugin::create(global, bun_jsc::BunPluginTarget::Bun);
let p = NonNull::new(p)
.expect("JSBundlerPlugin__create returns a non-null protected JSCell");
self.plugin = Some(p);
p
}
};
let plugin: &mut OwnedPlugin = self
.plugin
.get_or_insert_with(|| OwnedPlugin::create(global, bun_jsc::BunPluginTarget::Bun));
let empty_object = JSValue::create_empty_object(global, 0);

let mut iter = plugin_array.array_iterator(global)?;
Expand Down Expand Up @@ -356,15 +335,8 @@ impl SplitBundlerOptions {
}
};

// `Plugin` is an `opaque_ffi!` ZST — `opaque_mut` is the safe
// deref. Handle held live in `self.plugin` (protected JSCell).
let plugin_result = Plugin::opaque_mut(plugin.as_ptr()).add_plugin(
function,
empty_object,
JSValue::NULL,
false,
true,
)?;
let plugin_result =
plugin.add_plugin(function, empty_object, JSValue::NULL, false, true)?;

if let Some(promise) = plugin_result.as_any_promise() {
promise.set_handled(global.vm());
Expand Down
32 changes: 26 additions & 6 deletions src/runtime/bake/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
use core::ptr::NonNull;
use std::borrow::Cow;

use crate::api::js_bundler::OwnedPlugin;

// ─── Submodule bodies ────────────────────────────────────────────────────────
// `bake_body.rs` carries the Framework/UserOptions/BuildConfigSubset `from_js`
// impls plus the `init_server_runtime`/`get_hmr_runtime` host fns.
Expand Down Expand Up @@ -463,12 +465,32 @@ impl Framework {
}
}

/// The plugin cell every bundle of a `DevServer` runs against, and whether
/// the dev server is the one releasing it.
pub(crate) enum DevServerPlugin {
/// `framework.plugins` / `app.plugins`: created for this dev server by
/// `UserOptions::from_js` and released when the dev server drops.
Owned(OwnedPlugin),
/// The server's `[serve.static]` plugins, used when the app declares
/// none. The cell belongs to the server's `ServePlugins`, released by the
/// same server that owns this dev server; `DevServer::on_plugins_resolved`
/// only stores the pointer.
Borrowed(NonNull<jsc::Plugin>),
}

impl DevServerPlugin {
pub(crate) fn as_non_null(&self) -> NonNull<jsc::Plugin> {
match self {
DevServerPlugin::Owned(plugin) => plugin.as_non_null(),
DevServerPlugin::Borrowed(plugin) => *plugin,
}
}
}

/// `bake.SplitBundlerOptions` — per-graph bundler config + shared plugin.
#[derive(Default)]
pub struct SplitBundlerOptions {
/// FFI: `jsc.API.JSBundler.Plugin` (`JSBundlerPlugin__create`); deinit
/// goes through the C++ side. See LIFETIMES.tsv.
pub(crate) plugin: Option<NonNull<jsc::Plugin>>,
pub(crate) plugin: Option<DevServerPlugin>,
pub(crate) client: BuildConfigSubset,
pub(crate) server: BuildConfigSubset,
pub(crate) ssr: BuildConfigSubset,
Expand Down Expand Up @@ -561,9 +583,7 @@ impl From<bake_body::BuildConfigSubset> for BuildConfigSubset {
impl From<bake_body::SplitBundlerOptions> for SplitBundlerOptions {
fn from(src: bake_body::SplitBundlerOptions) -> Self {
Self {
// `bake_body::Plugin` and keystone `jsc::Plugin` both alias
// `crate::api::js_bundler::Plugin` — same nominal type, no cast.
plugin: src.plugin,
plugin: src.plugin.map(DevServerPlugin::Owned),
client: src.client.into(),
server: src.server.into(),
ssr: src.ssr.into(),
Expand Down
6 changes: 5 additions & 1 deletion src/runtime/bake/production.rs
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,11 @@ fn build_with_vm(ctx: Context, cwd: &[u8], pt: &mut PerThread) -> crate::Result<
framework: bundler_framework,
client_transpiler: NonNull::new(client_ptr).expect("stack-owned transpiler"),
ssr_transpiler: NonNull::new(ssr_ptr).expect("stack-owned transpiler"),
plugins: options.bundler_options.plugin,
plugins: options
.bundler_options
.plugin
.as_ref()
.map(|plugin| plugin.as_non_null()),
},
&options.arena,
Some(NonNull::from(&mut any_loop)),
Expand Down
57 changes: 57 additions & 0 deletions test/bake/app-plugins-release.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { expect, test } from "bun:test";
import { bunEnv, bunExe } from "harness";
import path from "node:path";

// `app.plugins` / `framework.plugins` are held by one native BundlerPlugin cell
// that the dev server takes over from the parsed serve options. The dev server
// used to drop its handle to that cell without releasing it, so every dev
// server created with plugins kept its cell, and every closure the plugins had
// registered, rooted for the rest of the process; options rejected after the
// cell had been created leaked it the same way. The fixture runs one of these
// paths per process and reports how many cells survive a full GC afterwards.
//
// The cases run one at a time: each dev server needs a file watcher instance,
// which Linux hands out per user, and a process only gives it back on exit.
// The fixture waits for one when the machine is out of them (hence the
// timeouts), and the debug build's startup plus full collections are slow.
async function runCase(name: string) {
await using proc = Bun.spawn({
cmd: [bunExe(), "app-plugins-release.ts", name],
cwd: path.join(import.meta.dir, "fixtures/app-plugins-release"),
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
// stderr is not asserted: "init-fails" makes Bun print the framework file it
// could not resolve there.
expect(stdout, stderr).toStartWith("{");
return { ...JSON.parse(stdout), exitCode };
}

test("a stopped dev server releases the cell holding its app plugins", async () => {
expect(await runCase("stopped-server")).toEqual({
cellsWhileServing: 1,
devServersDeinitialized: 1,
leakedCells: 0,
exitCode: 0,
});
}, 60_000);

test("serve options rejected by a plugin's setup() release the cell", async () => {
expect(await runCase("setup-throws")).toEqual({
error: "setup failed on purpose",
devServersDeinitialized: 0,
leakedCells: 0,
exitCode: 0,
});
}, 60_000);

test("a dev server that fails to initialize releases the cell", async () => {
expect(await runCase("init-fails")).toEqual({
error: "Framework is missing required files!",
devServersDeinitialized: 1,
leakedCells: 0,
exitCode: 0,
});
}, 60_000);
Loading
Loading