diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index ca925367b68a..0888dc086c74 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -1195,7 +1195,7 @@ impl VirtualMachine { + self.active_tasks + el.tasks.readable_length() + el.yield_tasks.len() - + (!el.concurrent_tasks.is_empty() as usize) + + (el.has_concurrent_tasks() as usize) + (el.has_pending_refs() as usize) > 0) } diff --git a/src/jsc/VmHandle.rs b/src/jsc/VmHandle.rs index c8a03dc8db22..b5a3f748999d 100644 --- a/src/jsc/VmHandle.rs +++ b/src/jsc/VmHandle.rs @@ -74,6 +74,8 @@ enum State { #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum LoopKind { Regular, + /// Once the macro has returned, the regular loop services this loop's + /// queue and keep-alive deltas too (`EventLoop::finished_macro_loop`). Macro, } diff --git a/src/jsc/event_loop.rs b/src/jsc/event_loop.rs index 6437e63f9456..621fc211c666 100644 --- a/src/jsc/event_loop.rs +++ b/src/jsc/event_loop.rs @@ -13,6 +13,7 @@ use core::ptr::NonNull; use core::sync::atomic::{AtomicI32, AtomicPtr, Ordering}; use bun_io::{self as Async, Waker}; +use bun_threading::unbounded_queue::Batch; use bun_uws as uws; use crate::js_promise::Status as PromiseStatus; @@ -479,9 +480,12 @@ impl EventLoop { } /// Whether a keep-alive delta (`ref_keep_alive`, here or through a - /// `VmHandle`) has been queued but not yet applied to the loop's `active` count. + /// `Ticket`) has been queued but not yet applied to the loop's `active` count. pub fn has_pending_refs(&self) -> bool { self.concurrent_ref.load(Ordering::SeqCst) > 0 + || self + .finished_macro_loop() + .is_some_and(|macro_loop| macro_loop.concurrent_ref.load(Ordering::SeqCst) > 0) } pub fn run_imminent_gc_timer(&mut self) { @@ -513,47 +517,51 @@ impl EventLoop { self.run_imminent_gc_timer(); - let concurrent = self.concurrent_tasks.pop_batch(); - let count = concurrent.count; - if count == 0 { - return 0; - } - - let mut iter = concurrent.iterator(); let start_count = self.tasks.readable_length(); - let _ = self.tasks.ensure_unused_capacity(count); - - // Defer destruction of the ConcurrentTask to avoid issues with pointer aliasing - let mut to_destroy: Option<*mut ConcurrentTaskItem> = None; - - loop { - let task = iter.next(); - if task.is_null() { - break; - } - if let Some(dest) = to_destroy.take() { - // SAFETY: dest was returned by iterator and marked auto_delete; uniquely owned here - let _ = unsafe { bun_core::heap::take(dest) }; - } - - // SAFETY: `task` is non-null (checked above) and owned by this - // batch; only shared reads follow (`auto_delete`, the `task` copy). - let task_ref = unsafe { &*task }; - if task_ref.auto_delete() { - to_destroy = Some(task); - } - - // LinearFifo's fields are private — `write_item` is the - // public path (single-slot copy, same complexity). - let _ = self.tasks.write_item(task_ref.task); + let posted = self.concurrent_tasks.pop_batch(); + self.take_concurrent_batch(posted); + if let Some(macro_loop) = self.finished_macro_loop() { + macro_loop.apply_concurrent_ref_delta(); + let posted = macro_loop.concurrent_tasks.pop_batch(); + self.take_concurrent_batch(posted); } + self.tasks.readable_length() - start_count + } - if let Some(dest) = to_destroy { - // SAFETY: see above - let _ = unsafe { bun_core::heap::take(dest) }; + /// Move a popped batch into `self.tasks`, freeing the heap `ConcurrentTask` carriers. + fn take_concurrent_batch(&mut self, batch: Batch) { + if batch.count == 0 { + return; + } + let _ = self.tasks.ensure_unused_capacity(batch.count); + let mut iter = batch.iterator(); + while let Some(node) = NonNull::new(iter.next()) { + // SAFETY: a node of the popped batch; the iterator has already moved past it. + let task = unsafe { ConcurrentTaskItem::into_task(node) }; + let _ = self.tasks.write_item(task); } + } - self.tasks.readable_length() - start_count + /// The macro loop, when this is the regular loop and no macro is running. + /// Work a macro started posts its completion and its keep-alive release + /// there (`LoopKind::Macro`), but that loop only ticks while a macro is + /// being waited on, so whatever finishes after the macro returned would + /// otherwise sit there forever, and the platform loop both loops share + /// would stay alive for it. + fn finished_macro_loop(&self) -> Option<&EventLoop> { + let vm = self.vm_ref(); + (vm.has_enabled_macro_mode + && !vm.macro_mode + && core::ptr::eq(self, &raw const vm.regular_event_loop)) + .then_some(&vm.macro_event_loop) + } + + /// Other threads have posted work that this loop's next drain will pick up. + pub(crate) fn has_concurrent_tasks(&self) -> bool { + !self.concurrent_tasks.is_empty() + || self + .finished_macro_loop() + .is_some_and(|macro_loop| !macro_loop.concurrent_tasks.is_empty()) } /// Fold refs/unrefs queued through `ref_keep_alive`/`unref_keep_alive` @@ -643,7 +651,7 @@ impl EventLoop { /// Work is queued that the next `tick()` will run: the poll before it must /// not block. pub fn has_pending_tasks(&self) -> bool { - self.tasks.readable_length() > 0 || !self.concurrent_tasks.is_empty() + self.tasks.readable_length() > 0 || self.has_concurrent_tasks() } pub fn tick(&mut self) { @@ -730,27 +738,6 @@ impl EventLoop { let _ = self.tasks.write_item(task); } - /// Move whatever other threads posted (`concurrent_tasks`) into - /// `self.tasks`, freeing the heap `ConcurrentTask` carriers, so one pass - /// over `self.tasks` releases everything. Called by `release_queued_tasks` - /// in teardown, after `join_child_workers()` (every child has posted its - /// close task by then) and before the JSC VM is destroyed (so captured - /// `Ref<>`s in queued C++ lambdas drop against a live heap). - fn take_concurrent_tasks(&mut self) { - let mut iter = self.concurrent_tasks.pop_batch().iterator(); - loop { - let node = iter.next(); - if node.is_null() { - break; - } - // SAFETY: `node` is non-null and owned by the popped batch; the - // iterator advanced past it before returning. - let task = - unsafe { ConcurrentTask::ConcurrentTask::into_task(NonNull::new_unchecked(node)) }; - let _ = self.tasks.write_item(task); - } - } - /// Release, without running, every task still queued — what other /// threads posted and what this thread enqueued — through each type's /// `Taskable::release_unrun`, and refuse (release on arrival) anything @@ -759,7 +746,8 @@ impl EventLoop { /// once more after `Closed`. pub fn release_queued_tasks(&mut self) { self.closed_for_tasks = true; - self.take_concurrent_tasks(); + let posted = self.concurrent_tasks.pop_batch(); + self.take_concurrent_batch(posted); let _ = self.promote_yield_tasks(); while let Some(task) = self.tasks.read_item() { // SAFETY: JS thread, heap alive; `task` just left the queue. diff --git a/test/bundler/transpiler/macro-test.test.ts b/test/bundler/transpiler/macro-test.test.ts index 50bac3b40ede..3d68a40d32ae 100644 --- a/test/bundler/transpiler/macro-test.test.ts +++ b/test/bundler/transpiler/macro-test.test.ts @@ -182,6 +182,85 @@ test("object destructuring of a macro result keeps every bound property regardle expect(exitCode).toBe(0); }); +// Async work a macro starts without awaiting it posts its completion (and, for WebCrypto, the release +// of its keep-alive) to the loop that was current when the work started, the macro loop, which stops +// ticking as soon as the macro returns. Work that is still pending at that point (always the case for +// work started inside the macro call itself) has to be picked up by the regular event loop from there. +// When it was not, the work kept the process alive forever: `bun run` printed the value and then never +// exited, which here shows up as the child never exiting. The top-level case usually finishes while +// the macro module is still being loaded and covers the continuation running from the macro loop +// itself. +describe("work a macro starts without awaiting it", () => { + const cases: [name: string, macroSource: string][] = [ + [ + "an fs.promises call inside the macro", + [ + `import { promises as fs } from "node:fs";`, + `export function m() {`, + ` fs.stat(import.meta.dir).then(() => console.log("settled"));`, + ` return 1;`, + `}`, + ].join("\n"), + ], + [ + "an fs.promises call at the macro module's top level", + [ + `import { promises as fs } from "node:fs";`, + `fs.stat(import.meta.dir).then(() => console.log("settled"));`, + `export function m() {`, + ` return 1;`, + `}`, + ].join("\n"), + ], + [ + "a fetch() inside the macro", + [ + `export function m() {`, + ` fetch(process.env.MACRO_TEST_URL!).then(res => res.text()).then(body => console.log(body));`, + ` return 1;`, + `}`, + ].join("\n"), + ], + [ + // Inputs of 64 bytes or more are hashed on the WebCrypto work queue, which releases its keep-alive + // through the macro loop rather than completing through a thread-pool job like the cases above. + "a crypto.subtle.digest() inside the macro", + [ + `export function m() {`, + ` crypto.subtle.digest("SHA-256", new Uint8Array(4096)).then(() => console.log("settled"));`, + ` return 1;`, + `}`, + ].join("\n"), + ], + ]; + + test.concurrent.each(cases)("%s still lets the process exit", async (_name, macroSource) => { + await using server = Bun.serve({ port: 0, fetch: () => new Response("settled") }); + using dir = tempDir("macro-unawaited-work", { + "m.ts": macroSource, + "index.ts": `import { m } from "./m.ts" with { type: "macro" };\nconsole.log("value", m());\n`, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "run", "index.ts"], + env: { ...bunEnv, MACRO_TEST_URL: server.url.href }, + cwd: String(dir), + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + // Debug builds also print "[macro] call m". The continuation runs on the macro loop if the work + // finishes while the macro is still being waited on, and on the regular loop otherwise, so its + // position relative to the entry module's own output varies; hence the sort. + const lines = stdout + .trim() + .split("\n") + .filter(line => !line.startsWith("[macro]")) + .sort(); + expect({ lines, stderr }).toEqual({ lines: ["settled", "value 1"], stderr: "" }); + expect(exitCode).toBe(0); + }); +}); + describe("--no-macros", () => { const files = { "macro.ts": `