Skip to content
Merged
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
19 changes: 17 additions & 2 deletions src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4204,8 +4204,12 @@ impl VirtualMachine {
let mut log = bun_ast::Log::default();
jsc_vm.log = NonNull::new(&raw mut log);
jsc_vm.transpiler.resolver.log = NonNull::from(&mut log);
// TODO(b2-cycle): `transpiler.linker.log` / `resolver.package_manager.log`
// — gated bundler fields.
jsc_vm.transpiler.linker.log = &raw mut log;
if let Some(pm) = jsc_vm.transpiler.resolver.package_manager {
// SAFETY: the `dyn AutoInstaller` is always `PackageManager`
// (sole impl — see `VirtualMachine::package_manager`).
unsafe { (*pm.cast::<bun_install::PackageManager>().as_ptr()).log = &raw mut log };
}
// PORT NOTE: Zig `defer { restore old_log }` — fires on every exit
// (including `?` from `ResolveMessage::create` below), so the VM's
// `log` cannot be left pointing at the dropped stack `log`. Hand-roll
Expand All @@ -4223,6 +4227,17 @@ impl VirtualMachine {
let jsc_vm = self.vm.get().as_mut();
jsc_vm.log = Some(self.old_log);
jsc_vm.transpiler.resolver.log = self.old_log;
jsc_vm.transpiler.linker.log = self.old_log.as_ptr();
// `_resolve` may have lazily created the PM with
// `pm.log = resolver.log` (our stack `log`), so restore even
// if it was `None` when we swapped.
if let Some(pm) = jsc_vm.transpiler.resolver.package_manager {
// SAFETY: sole `dyn AutoInstaller` impl is `PackageManager`.
unsafe {
(*pm.cast::<bun_install::PackageManager>().as_ptr()).log =
self.old_log.as_ptr();
}
}
}
}
let _restore = RestoreLog {
Expand Down
1 change: 1 addition & 0 deletions src/jsc/bindings/wtf-bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#if !OS(WINDOWS)
#include <stdatomic.h>

#include <cassert>
#include <signal.h>
#include <termios.h>
static int orig_termios_fd = -1;
Expand Down
41 changes: 17 additions & 24 deletions src/runtime/jsc_hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2157,31 +2157,18 @@ fn transpile_source_code_inner(
let args_log_nn = core::ptr::NonNull::new(args.log).expect("args.log is non-null");
unsafe {
(*jsc_vm).transpiler.log = args.log;
{
(*jsc_vm).transpiler.resolver.log = args_log_nn;
}
// TODO(b2-blocked): `Linker` is a unit stub in `bun_bundler`
// — `.log` field un-gates with `linker.rs`.

{
(*jsc_vm).transpiler.linker.log = args.log;
if let Some(pm) = (*jsc_vm).transpiler.resolver.package_manager {
// TODO(blocked_on): bun_resolver::package_json::PackageManager::log
// — the resolver-side stub only exposes `lockfile`/`on_wake`.
let _ = pm;
}
(*jsc_vm).transpiler.resolver.log = args_log_nn;
(*jsc_vm).transpiler.linker.log = args.log;
if let Some(pm) = (*jsc_vm).transpiler.resolver.package_manager {
(*pm.cast::<bun_install::PackageManager>().as_ptr()).log = args.log;
}
}
let _log_guard = scopeguard::guard(jsc_vm, move |jsc_vm| unsafe {
(*jsc_vm).transpiler.log = old_log;

{
(*jsc_vm).transpiler.resolver.log = old_log_nn;
(*jsc_vm).transpiler.linker.log = old_log;
if let Some(pm) = (*jsc_vm).transpiler.resolver.package_manager {
// TODO(blocked_on): bun_resolver::package_json::PackageManager::log
let _ = pm;
}
(*jsc_vm).transpiler.resolver.log = old_log_nn;
(*jsc_vm).transpiler.linker.log = old_log;
if let Some(pm) = (*jsc_vm).transpiler.resolver.package_manager {
(*pm.cast::<bun_install::PackageManager>().as_ptr()).log = old_log;
}
});

Expand Down Expand Up @@ -4943,17 +4930,23 @@ unsafe fn resolve_hook(
(*vm).log = Some(log_nn);
(*vm).transpiler.resolver.log = log_nn;
(*vm).transpiler.linker.log = log_nn.as_ptr();
// TODO(b2-cycle): `transpiler.resolver.package_manager` log swap —
// gated alongside the PM field (see transpile_source_code §log-swap).
if let Some(pm) = (*vm).transpiler.resolver.package_manager {
(*pm.cast::<bun_install::PackageManager>().as_ptr()).log = log_nn.as_ptr();
}
Comment thread
claude[bot] marked this conversation as resolved.
}
scopeguard::defer! {
// SAFETY: `vm` is the live per-thread VM; restoring the log pointers
// swapped just above so early-return paths don't leave a dangling
// stack pointer.
// stack pointer. The PM may have been lazily created inside
// `_resolve` with `pm.log = resolver.log` (our stack `log`), so
// restore it even if it was `None` at swap time.
unsafe {
(*vm).log = Some(old_log);
(*vm).transpiler.resolver.log = old_log;
(*vm).transpiler.linker.log = old_log.as_ptr();
if let Some(pm) = (*vm).transpiler.resolver.package_manager {
(*pm.cast::<bun_install::PackageManager>().as_ptr()).log = old_log.as_ptr();
}
}
}

Expand Down
59 changes: 59 additions & 0 deletions test/js/bun/resolve/resolve-autoinstall-log-dangling.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { expect, test } from "bun:test";
import { bunEnv, bunExe, tempDir } from "harness";

// When the runtime resolver's auto-install path lazily creates the
// PackageManager, it snapshots `resolver.log` into `pm.log`. If the
// PackageManager is created while the resolver log is pointed at a
// stack-local `Log` inside `resolveMaybeNeedsTrailingSlash`, `pm.log` must be
// restored to the VM log before returning, otherwise the next resolve at a
// different stack depth dereferences dead stack memory when the HTTP task
// fails and calls `manager.log_mut().add_error_fmt(...)`.
test("repeated failing auto-install resolves at varying stack depth don't read a dangling pm.log", async () => {
// Run from an empty dir so the resolver falls through to the auto-install
// path instead of stopping at the repo's own node_modules.
using dir = tempDir("resolve-autoinstall-log", {});

const src = `
const realm = new ShadowRealm();
const variants = [
() => realm.importValue("pkg-not-found-a", "x"),
() => (() => realm.importValue("pkg-not-found-b", "x"))(),
() => (() => (() => realm.importValue("pkg-not-found-c", "x"))())(),
() => { try { return realm.importValue("pkg-not-found-d", "x"); } finally {} },
() => [realm.importValue("pkg-not-found-e", "x")][0],
() => import("pkg-not-found-f"),
() => (async () => realm.importValue("pkg-not-found-g", "x"))(),
];
for (let i = 0; i < 100; i++) {
for (const v of variants) {
try {
const p = v();
if (p && p.catch) p.catch(() => {});
} catch {}
}
}
Bun.gc(true);
console.log("ok");
`;

await using proc = Bun.spawn({
// Force the auto-install path (PackageManager lazy init inside the
// resolver) without hitting the network — the registry hostname is
// unroutable, so every enqueued manifest fetch fails fast and routes
// through `manager.log_mut()`.
cmd: [bunExe(), "--install=fallback", "-e", src],
cwd: String(dir),
env: {
...bunEnv,
BUN_CONFIG_REGISTRY: "http://127.0.0.1:1",
},
stdout: "pipe",
stderr: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

expect(stderr).toBe("");
expect(stdout.trim()).toBe("ok");
expect(exitCode).toBe(0);
});
Loading