Skip to content
Open
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
2 changes: 1 addition & 1 deletion src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 2 additions & 0 deletions src/jsc/VmHandle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`).
Comment thread
robobun marked this conversation as resolved.
Macro,
}

Expand Down
108 changes: 48 additions & 60 deletions src/jsc/event_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
Comment thread
claude[bot] marked this conversation as resolved.
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<ConcurrentTaskItem>) {
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.
Comment thread
robobun marked this conversation as resolved.
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`
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down
79 changes: 79 additions & 0 deletions test/bundler/transpiler/macro-test.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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": `
Expand Down