diff --git a/src/bundler/transpiler.rs b/src/bundler/transpiler.rs index 536e3ea26598..43694e7fac8c 100644 --- a/src/bundler/transpiler.rs +++ b/src/bundler/transpiler.rs @@ -309,8 +309,9 @@ impl<'a> Transpiler<'a> { /// lifetime-carrying borrows in `BundleOptions<'_>` / `Resolver<'_>` /// (`framework`, `optimize_imports`, `standalone_module_graph`, /// `env_loader`) are widened from `from`'s lifetime to `'a` via a - /// layout-preserving transmute — sound because those reference - /// process-lifetime data in every caller, but unprovable to borrowck. + /// layout-preserving transmute — sound because what they point at + /// outlives `from` in every caller (process-lifetime singletons, or for + /// `env_loader` the build that owns `from`), but unprovable to borrowck. pub(crate) unsafe fn for_worker( from: &Transpiler<'_>, arena: &'a Arena, diff --git a/src/dotenv/env_loader.rs b/src/dotenv/env_loader.rs index 57b4464602a8..a1f9370f8652 100644 --- a/src/dotenv/env_loader.rs +++ b/src/dotenv/env_loader.rs @@ -595,6 +595,21 @@ impl Loader { } } + /// Copies the variables and the loaded-files bookkeeping (so `load_process` + /// / `load` on the copy are the same no-ops as on `self`); the lazily + /// derived caches are rebuilt from the copied map on demand. + pub fn clone(&self) -> Result { + Ok(Loader { + map: self.map.clone_with_allocator()?, + default_files_loaded: self.default_files_loaded, + custom_files_loaded: self.custom_files_loaded.clone()?, + quiet: self.quiet, + did_load_process: self.did_load_process, + reject_unauthorized: Cell::new(None), + aws_credentials: None, + }) + } + pub fn load_process(&mut self) -> Result<(), AllocError> { if self.did_load_process { return Ok(()); diff --git a/src/js_parser_jsc/Macro.rs b/src/js_parser_jsc/Macro.rs index 4d6e8a203f53..a6628dbc52c8 100644 --- a/src/js_parser_jsc/Macro.rs +++ b/src/js_parser_jsc/Macro.rs @@ -423,17 +423,31 @@ impl Macro { // `RuntimeHooks::init_runtime_state` builds the macro VM's // transpiler from a fresh `TransformOptions` value rather than // borrowing the caller's, so there is nothing to mutate-and-restore - // on `resolver.opts` here. `log`/`env_loader` *are* threaded so the - // CLI-path macro VM uses the caller's log sink and env loader. + // on `resolver.opts` here. `log` *is* threaded so the CLI-path + // macro VM uses the caller's log sink. // JSC needs to be initialized if building from CLI jsc::initialize(false); + // The VM outlives this build (later builds' macros reuse it; it is + // never destroyed) and `env` does not, so the VM gets its own copy. + let env_loader = NonNull::new(env).map(|env| { + // SAFETY: the caller's loader, live and unwritten during this build. + let copy = bun_core::handle_oom(unsafe { env.as_ref() }.clone()); + bun_core::heap::into_raw_nn(Box::new(copy)) + }); let _vm = VirtualMachine::init(VirtualMachineInitOptions { log: Some(NonNull::from(&mut *log)), - env_loader: NonNull::new(env), + env_loader, is_main_thread: false, ..Default::default() + }) + .inspect_err(|_| { + if let Some(copy) = env_loader { + // SAFETY: a failed `init` never wrote the transpiler that + // would have kept this pointer, so the copy is still ours. + unsafe { bun_core::heap::destroy(copy.as_ptr()) }; + } })?; (_vm, true) }; diff --git a/src/runtime/api/js_bundle_completion_task.rs b/src/runtime/api/js_bundle_completion_task.rs index 27657f0da198..0c538eca2506 100644 --- a/src/runtime/api/js_bundle_completion_task.rs +++ b/src/runtime/api/js_bundle_completion_task.rs @@ -59,7 +59,10 @@ pub struct JSBundleCompletionTask { pub global_this: BackRef, pub(crate) promise: jsc::JSPromiseStrong, pub poll_ref: KeepAlive, - pub(crate) env: *mut bun_dotenv::Loader, + /// This build's copy of the calling VM's env, read on the bundle thread + /// while the VM's own loader keeps changing on the JS thread + /// (`Bun__setEnvValue`). Boxed: the build's transpiler holds a pointer to it. + pub(crate) env: Box, pub(crate) log: bun_ast::Log, /// Set by the owner giving up on the result (HTMLBundle route torn down) /// or by the VM's stop phase; read by `on_complete` (skip delivery) and by @@ -120,7 +123,7 @@ impl JSBundleCompletionTask { // last-ref drop is the only place that releases it. Plugin::destroy(plugin.as_ptr()); } - // Owned fields (`config`, `log`, `result`, `promise`) drop with the Box. + // Owned fields (`config`, `env`, `log`, `result`, `promise`) drop with the Box. } } @@ -141,7 +144,11 @@ pub(crate) fn create_and_schedule_completion_task( global_this: &JSGlobalObject, ) -> crate::Result<*mut JSBundleCompletionTask> { let vm = global_this.bun_vm_ptr(); - let env = global_this.bun_vm().transpiler.env; + // Same thread as `Bun__setEnvValue`, so no `proxy_env_storage` lock (unlike + // web_worker.rs). An error return here would leak `plugins`, so OOM aborts. + let env = Box::new(bun_core::handle_oom( + global_this.bun_vm().env_loader().clone(), + )); let completion = bun_core::heap::into_raw(Box::new(JSBundleCompletionTask { ref_count: RefCount::init(), config, @@ -173,8 +180,8 @@ pub(crate) fn create_and_schedule_completion_task( let _ = WorkPool::get(); // Out on the bundle thread from here until it posts the completion: it - // reads this VM's env loader and the plugin cell, so the VM cancels it at - // teardown (registry) and waits for it (embedded work). + // reads this VM's plugin cell and posts into this VM's queue, so the VM + // cancels it at teardown (registry) and waits for it (embedded work). // SAFETY: `completion` is live (refcount==1), JS thread. unsafe { (*completion).loop_handle.embedded_work_scheduled() }; crate::jsc_hooks::ActiveHandle::Bundle(NonNull::new(completion).expect("completion")) @@ -426,10 +433,7 @@ impl JSBundleCompletionTask { root_dir.fd, module_prefix, outfile_for_executable, - // SAFETY: `self.env` is the per-VM `DotEnv.Loader` stashed at - // construction; valid for the lifetime of the VirtualMachine, and - // nothing inside `to_executable` reaches it otherwise. - unsafe { &mut *self.env }, + &mut self.env, self.config.format, &WindowsOptions { hide_console: compile_options.windows_hide_console, @@ -909,9 +913,9 @@ impl CompletionStruct for JSBundleCompletionTask { core::hint::spin_loop(); } // The VM released everything thread-affine (`stop_for_vm_teardown`); - // what is left — config, log, an empty promise slot, a `Done` - // keep-alive, the handle clone — is ours to drop here. The queue held - // the creation reference. + // what is left (config, the env copy, log, an empty promise slot, a + // `Done` keep-alive, the handle clone) is ours to drop here. The + // queue held the creation reference. // SAFETY: dequeued ⇒ sole owner; nothing JS-affine remains. drop(unsafe { bun_core::heap::take(this) }); } @@ -1105,8 +1109,7 @@ impl CompletionStruct for JSBundleCompletionTask { .conditions .append_slice(&[b"development"])?; } - // `transpiler.env` is the dotenv loader installed by - // `Transpiler::init`; non-null and valid for `'a`. + // `transpiler.env` is this build's copy (`self.env`), not the VM's loader. transpiler.resolver.env_loader = NonNull::new(transpiler.env); // `Resolver.opts` is the resolver-crate subset // — re-project from the now-mutated `transpiler.options`. @@ -1191,7 +1194,10 @@ impl CompletionStruct for JSBundleCompletionTask { }; let log: *mut bun_ast::Log = &raw mut self.log; - let t = Transpiler::init(bump, log, opts, Some(self.env))?; + // Freed with `self`: read only until `complete_on_bundle_thread`, except + // by the per-thread macro VM, which copies it (`Macro::init`). + let env: *mut bun_dotenv::Loader = &raw mut *self.env; + let t = Transpiler::init(bump, log, opts, Some(env))?; let transpiler: &'a mut Transpiler<'a> = bump.alloc(t); // Post-init field wiring. diff --git a/test/bundler/bundler_env.test.ts b/test/bundler/bundler_env.test.ts index b87ab47ab605..f6d22c65a3aa 100644 --- a/test/bundler/bundler_env.test.ts +++ b/test/bundler/bundler_env.test.ts @@ -1,4 +1,5 @@ -import { describe } from "bun:test"; +import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; import { itBundled } from "./expectBundled"; for (let backend of ["api", "cli"] as const) { @@ -118,3 +119,144 @@ for (let backend of ["api", "cli"] as const) { }); }); } + +// A build inlines the env as of the call that started it. The bundler thread +// reads env from its own copy: `process.env.HTTPS_PROXY = ...` (one of the +// few assignments that write through to the native env map, replacing the +// stored value in place) while a build is in flight must not change, or free +// out from under, what the build inlines. +describe.concurrent("env is copied when the build is scheduled", () => { + const atCall = "http://proxy-when-the-build-was-scheduled.example:1111/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; + const duringBuild = "http://proxy-assigned-while-bundling.example:2222/bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; + + // The plugin's onLoad runs on the JS thread while the bundle is in flight, + // after the bundler has built its defines. + const pluginSource = (filter: string) => /* ts */ ` + export default { + name: "assign-proxy-env-while-bundling", + setup(build) { + build.onLoad({ filter: ${filter} }, () => { + process.env.HTTPS_PROXY = ${JSON.stringify(duringBuild)}; + return { loader: "ts", contents: "console.log(process.env.HTTPS_PROXY);" }; + }); + }, + }; + `; + + test.each(["inline", "HTTPS_*"] as const)("Bun.build({ env: %j })", async env => { + using dir = tempDir("bun-build-env-copy", { + "entry.ts": "export {};", + "plugin.ts": pluginSource("/entry\\.ts$/"), + "build-fixture.ts": /* ts */ ` + import plugin from "./plugin.ts"; + process.env.HTTPS_PROXY = ${JSON.stringify(atCall)}; + const result = await Bun.build({ + entrypoints: ["./entry.ts"], + env: ${JSON.stringify(env)}, + plugins: [plugin], + }); + process.stdout.write(await result.outputs[0].text()); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "build-fixture.ts"], + 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).toContain(`console.log(${JSON.stringify(atCall)})`); + expect(stdout).not.toContain(duringBuild); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); + }); + + // Bun.serve's HTML routes (without HMR) are built through the same bundler + // thread, with env behavior and plugins coming from bunfig. + test('Bun.serve HTML route with [serve.static] env = "inline"', async () => { + using dir = tempDir("bun-serve-html-env-copy", { + "bunfig.toml": /* toml */ ` + [serve.static] + env = "inline" + plugins = ["./plugin.ts"] + `, + "index.html": /* html */ ``, + "app.ts": "export {};", + "plugin.ts": pluginSource("/app\\.ts$/"), + "serve-fixture.ts": /* ts */ ` + import index from "./index.html"; + process.env.HTTPS_PROXY = ${JSON.stringify(atCall)}; + using server = Bun.serve({ + port: 0, + development: false, + routes: { "/": index }, + }); + const html = await (await fetch(server.url)).text(); + const script = html.match(/src="([^"]+\\.js)"/)![1]; + process.stdout.write(await (await fetch(new URL(script, server.url))).text()); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "serve-fixture.ts"], + 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).toContain(`console.log(${JSON.stringify(atCall)})`); + expect(stdout).not.toContain(duringBuild); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); + }); + + // Macros run in a VM that a bundler thread creates the first time a macro + // runs on it and then reuses for every later build, so it must not keep + // reading the env of the build that created it, which is freed with that + // build. Two bundler threads and four builds guarantee that later builds run + // their macro on a thread whose VM an earlier build created. + test("macros in later builds reuse a VM created during an earlier build", async () => { + const builds = 4; + const env: Record = { ...bunEnv, UV_THREADPOOL_SIZE: "2" }; + const files: Record = { + "macro.ts": /* ts */ `export function envValue(name: string) { return process.env[name]; }`, + "build-fixture.ts": /* ts */ ` + for (let i = 0; i < ${builds}; i++) { + const result = await Bun.build({ entrypoints: ["./entry" + i + ".ts"] }); + if (!result.success) throw new AggregateError(result.logs, "build " + i + " failed"); + process.stdout.write(await result.outputs[0].text()); + } + `, + }; + for (let i = 0; i < builds; i++) { + // Each build reads a key no earlier build has read, so the lookup goes + // to the VM's env instead of an already materialized process.env entry. + env[`MACRO_ENV_${i}`] = `value-of-build-${i}`; + files[`entry${i}.ts`] = /* ts */ ` + import { envValue } from "./macro.ts" with { type: "macro" }; + console.log(envValue("MACRO_ENV_${i}")); + `; + } + using dir = tempDir("bun-build-env-macro-vm", files); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "build-fixture.ts"], + env, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stdout.match(/console\.log\("(value-of-build-\d)"\)/g)).toEqual( + Array.from({ length: builds }, (_, i) => `console.log("value-of-build-${i}")`), + ); + expect(stderr).toBe(""); + expect(exitCode).toBe(0); + }); +});