Address PR review comments for runtime inspector

- Add PID validation in Windows tests to fail early on invalid PID file content
- Add diagnostic logging for Windows file mapping failures
- Remove log() call from signal handler context for async-signal-safety
- Remove unused previous_action variable from POSIX signal handler
This commit is contained in:
Alistair Smith
2026-01-05 17:08:08 +00:00
committed by Claude
parent 0ae67c72bc
commit cc6704de2f
2 changed files with 10 additions and 7 deletions

View File

@@ -29,10 +29,9 @@ var installed: std.atomic.Value(bool) = std.atomic.Value(bool).init(false);
var inspector_activation_requested: std.atomic.Value(bool) = std.atomic.Value(bool).init(false);
fn requestInspectorActivation() void {
const vm = VirtualMachine.getMainThreadVM() orelse {
log("No main thread VM available", .{});
return;
};
// Note: This function may be called from signal handler context on POSIX,
// so we must only use async-signal-safe operations here.
const vm = VirtualMachine.getMainThreadVM() orelse return;
inspector_activation_requested.store(true, .release);
vm.eventLoop().wakeup();
@@ -94,8 +93,6 @@ pub fn isInstalled() bool {
}
const posix = if (Environment.isPosix) struct {
var previous_action: std.posix.Sigaction = undefined;
fn signalHandler(_: c_int) callconv(.c) void {
// This handler runs in signal context, so we can only do async-signal-safe operations.
// Set the atomic flag and wake the event loop.
@@ -112,7 +109,7 @@ const posix = if (Environment.isPosix) struct {
.flags = std.posix.SA.RESTART,
};
std.posix.sigaction(std.posix.SIG.USR1, &act, &previous_action);
std.posix.sigaction(std.posix.SIG.USR1, &act, null);
return true;
}
@@ -211,11 +208,13 @@ const windows = if (Environment.isWindows) struct {
_ = UnmapViewOfFile(ptr);
return true;
} else {
log("MapViewOfFile failed", .{});
_ = bun.windows.CloseHandle(handle);
mapping_handle = null;
return false;
}
} else {
log("CreateFileMappingW failed for bun-debug-handler-{d}", .{pid});
return false;
}
}

View File

@@ -39,6 +39,7 @@ describe.skipIf(!isWindows)("Runtime inspector Windows file mapping", () => {
reader.releaseLock();
const pid = parseInt(await Bun.file(join(String(dir), "pid")).text(), 10);
expect(pid).toBeGreaterThan(0);
// Use _debugProcess which uses file mapping on Windows
await using debugProc = spawn({
@@ -130,6 +131,7 @@ describe.skipIf(!isWindows)("Runtime inspector Windows file mapping", () => {
reader.releaseLock();
const pid = parseInt(await Bun.file(join(String(dir), "pid")).text(), 10);
expect(pid).toBeGreaterThan(0);
// Set up stderr reader to wait for debugger to start
const stderrReader = targetProc.stderr.getReader();
@@ -225,6 +227,8 @@ describe.skipIf(!isWindows)("Runtime inspector Windows file mapping", () => {
const pid1 = parseInt(await Bun.file(join(String(dir), "pid-1")).text(), 10);
const pid2 = parseInt(await Bun.file(join(String(dir), "pid-2")).text(), 10);
expect(pid1).toBeGreaterThan(0);
expect(pid2).toBeGreaterThan(0);
// Activate inspector in both
await using debug1 = spawn({