Skip to content

Commit

Permalink
fix(windows spawn): use Job Object to manage subprocesses of subproce…
Browse files Browse the repository at this point in the history
…sses (#11240)

Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
  • Loading branch information
paperdave and Jarred-Sumner committed May 24, 2024
1 parent d1ac51e commit c3157e2
Show file tree
Hide file tree
Showing 12 changed files with 268 additions and 56 deletions.
2 changes: 1 addition & 1 deletion src/bun.js/WebKit
Submodule WebKit updated 1691 files
29 changes: 19 additions & 10 deletions src/bun.js/api/bun/process.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1671,7 +1671,7 @@ pub fn spawnProcessWindows(
uv_process_options.stdio_count = @intCast(stdio_containers.items.len);

uv_process_options.exit_cb = &Process.onExitUV;
var process = Process.new(.{
const process = Process.new(.{
.event_loop = options.windows.loop,
.pid = 0,
});
Expand Down Expand Up @@ -1759,24 +1759,28 @@ pub const sync = struct {
cwd: []const u8 = "",
detached: bool = false,

argv: []const []const u8 = &.{},
argv: []const []const u8,
/// null = inherit parent env
envp: ?[*:null]?[*:0]const u8,

use_execve_on_macos: bool = false,
argv0: ?[*:0]const u8 = null,

windows: if (Environment.isWindows) WindowsSpawnOptions.WindowsOptions else void = if (Environment.isWindows) .{} else undefined,
windows: if (Environment.isWindows) WindowsSpawnOptions.WindowsOptions else void = if (Environment.isWindows) .{},

pub const Stdio = union(enum) {
inherit: void,
ignore: void,
buffer: if (Environment.isWindows) *uv.Pipe else void,
pub const Stdio = enum {
inherit,
ignore,
buffer,

pub fn toStdio(this: *const Stdio) SpawnOptions.Stdio {
return switch (this.*) {
.inherit => .{ .inherit = this.inherit },
.ignore => .{ .ignore = this.ignore },
.buffer => .{ .buffer = this.buffer },
.inherit => .inherit,
.ignore => .ignore,
.buffer => .{
.buffer = if (Environment.isWindows)
bun.default_allocator.create(bun.windows.libuv.Pipe) catch bun.outOfMemory(),
},
};
}
};
Expand Down Expand Up @@ -1806,6 +1810,11 @@ pub const sync = struct {
pub fn isOK(this: *const Result) bool {
return this.status.isOK();
}

pub fn deinit(this: *const Result) void {
this.stderr.deinit();
this.stdout.deinit();
}
};

const SyncWindowsPipeReader = struct {
Expand Down
99 changes: 76 additions & 23 deletions src/bun.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2232,8 +2232,34 @@ pub const win32 = struct {
// this process will be the parent of the child process that actually runs the script
var procinfo: std.os.windows.PROCESS_INFORMATION = undefined;
C.windows_enable_stdio_inheritance();
const job = windows.CreateJobObjectA(null, null) orelse Output.panic(
"Could not create watcher Job Object: {s}",
.{@tagName(std.os.windows.kernel32.GetLastError())},
);
var jeli = std.mem.zeroes(windows.JOBOBJECT_EXTENDED_LIMIT_INFORMATION);
jeli.BasicLimitInformation.LimitFlags =
windows.JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE |
windows.JOB_OBJECT_LIMIT_BREAKAWAY_OK |
windows.JOB_OBJECT_LIMIT_SILENT_BREAKAWAY_OK |
windows.JOB_OBJECT_LIMIT_DIE_ON_UNHANDLED_EXCEPTION;
if (windows.SetInformationJobObject(
job,
windows.JobObjectExtendedLimitInformation,
&jeli,
@sizeOf(windows.JOBOBJECT_EXTENDED_LIMIT_INFORMATION),
) == 0) {
Output.panic(
"Could not configure watcher Job Object: {s}",
.{@tagName(std.os.windows.kernel32.GetLastError())},
);
}

while (true) {
spawnWatcherChild(allocator, &procinfo) catch |err| {
spawnWatcherChild(allocator, &procinfo, job) catch |err| {
handleErrorReturnTrace(err, @errorReturnTrace());
if (err == error.Win32Error) {
Output.panic("Failed to spawn process: {s}\n", .{@tagName(std.os.windows.kernel32.GetLastError())});
}
Output.panic("Failed to spawn process: {s}\n", .{@errorName(err)});
};
w.WaitForSingleObject(procinfo.hProcess, w.INFINITE) catch |err| {
Expand All @@ -2259,8 +2285,29 @@ pub const win32 = struct {
pub fn spawnWatcherChild(
allocator: std.mem.Allocator,
procinfo: *std.os.windows.PROCESS_INFORMATION,
job: w.HANDLE,
) !void {
const flags: std.os.windows.DWORD = w.CREATE_UNICODE_ENVIRONMENT;
// https://devblogs.microsoft.com/oldnewthing/20230209-00/?p=107812
var attr_size: usize = undefined;
_ = windows.InitializeProcThreadAttributeList(null, 1, 0, &attr_size);
const p = try allocator.alloc(u8, attr_size);
defer allocator.free(p);
if (windows.InitializeProcThreadAttributeList(p.ptr, 1, 0, &attr_size) == 0) {
return error.Win32Error;
}
if (windows.UpdateProcThreadAttribute(
p.ptr,
0,
windows.PROC_THREAD_ATTRIBUTE_JOB_LIST,
@ptrCast(&job),
@sizeOf(w.HANDLE),
null,
null,
) == 0) {
return error.Win32Error;
}

const flags: std.os.windows.DWORD = w.CREATE_UNICODE_ENVIRONMENT | windows.EXTENDED_STARTUPINFO_PRESENT;

const image_path = windows.exePathW();
var wbuf: WPathBuffer = undefined;
Expand Down Expand Up @@ -2298,25 +2345,28 @@ pub const win32 = struct {
envbuf[size + watcherChildEnv.len + 2] = 0;
envbuf[size + watcherChildEnv.len + 3] = 0;

var startupinfo = w.STARTUPINFOW{
.cb = @sizeOf(w.STARTUPINFOW),
.lpReserved = null,
.lpDesktop = null,
.lpTitle = null,
.dwX = 0,
.dwY = 0,
.dwXSize = 0,
.dwYSize = 0,
.dwXCountChars = 0,
.dwYCountChars = 0,
.dwFillAttribute = 0,
.dwFlags = w.STARTF_USESTDHANDLES,
.wShowWindow = 0,
.cbReserved2 = 0,
.lpReserved2 = null,
.hStdInput = std.io.getStdIn().handle,
.hStdOutput = std.io.getStdOut().handle,
.hStdError = std.io.getStdErr().handle,
var startupinfo = windows.STARTUPINFOEXW{
.StartupInfo = .{
.cb = @sizeOf(windows.STARTUPINFOEXW),
.lpReserved = null,
.lpDesktop = null,
.lpTitle = null,
.dwX = 0,
.dwY = 0,
.dwXSize = 0,
.dwYSize = 0,
.dwXCountChars = 0,
.dwYCountChars = 0,
.dwFillAttribute = 0,
.dwFlags = w.STARTF_USESTDHANDLES,
.wShowWindow = 0,
.cbReserved2 = 0,
.lpReserved2 = null,
.hStdInput = std.io.getStdIn().handle,
.hStdOutput = std.io.getStdOut().handle,
.hStdError = std.io.getStdErr().handle,
},
.lpAttributeList = p.ptr,
};
@memset(std.mem.asBytes(procinfo), 0);
const rc = w.kernel32.CreateProcessW(
Expand All @@ -2328,12 +2378,15 @@ pub const win32 = struct {
flags,
envbuf.ptr,
null,
&startupinfo,
@ptrCast(&startupinfo),
procinfo,
);
if (rc == 0) {
Output.panic("Unexpected error while reloading process\n", .{});
return error.Win32Error;
}
var is_in_job: w.BOOL = 0;
_ = windows.IsProcessInJob(procinfo.hProcess, job, &is_in_job);
assert(is_in_job != 0);
_ = std.os.windows.ntdll.NtClose(procinfo.hThread);
}
};
Expand Down
17 changes: 13 additions & 4 deletions src/cli.zig
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,12 @@ pub const Arguments = struct {
}
} else if (args.flag("--watch")) {
ctx.debug.hot_reload = .watch;
bun.auto_reload_on_crash = true;

// Windows applies this to the watcher child process.
// The parent process is unable to re-launch itself
if (!bun.Environment.isWindows)
bun.auto_reload_on_crash = true;

if (args.flag("--no-clear-screen")) {
bun.DotEnv.Loader.has_no_clear_screen_cli_flag = true;
}
Expand Down Expand Up @@ -1222,9 +1227,13 @@ pub const Command = struct {
}

if (comptime Environment.isWindows) {
if (global_cli_ctx.debug.hot_reload == .watch and !bun.isWatcherChild()) {
// this is noreturn
bun.becomeWatcherManager(allocator);
if (global_cli_ctx.debug.hot_reload == .watch) {
if (!bun.isWatcherChild()) {
// this is noreturn
bun.becomeWatcherManager(allocator);
} else {
bun.auto_reload_on_crash = true;
}
}
}

Expand Down
74 changes: 60 additions & 14 deletions src/crash_handler.zig
Original file line number Diff line number Diff line change
Expand Up @@ -741,9 +741,9 @@ pub fn printMetadata(writer: anytype) !void {
try writer.writeAll(metadata_version_line);
{
try writer.print("Args: ", .{});
var arg_chars_left: usize = 196;
var arg_chars_left: usize = if (bun.Environment.isDebug) 4096 else 196;
for (bun.argv, 0..) |arg, i| {
if (i != 0) try writer.writeAll(", ");
if (i != 0) try writer.writeAll(" ");
try bun.fmt.quotedWriter(writer, arg[0..@min(arg.len, arg_chars_left)]);
arg_chars_left -|= arg.len;
if (arg_chars_left == 0) {
Expand Down Expand Up @@ -1009,7 +1009,7 @@ const StackLine = struct {
};

const TraceString = struct {
trace: *std.builtin.StackTrace,
trace: *const std.builtin.StackTrace,
reason: CrashReason,
action: Action,

Expand Down Expand Up @@ -1361,6 +1361,16 @@ const stdDumpStackTrace = debug.dumpStackTrace;
/// cases where such logic fails to run.
pub fn dumpStackTrace(trace: std.builtin.StackTrace) void {
const stderr = std.io.getStdErr().writer();
if (!bun.Environment.isDebug) {
// debug symbols aren't available, lets print a tracestring
stderr.print("View Debug Trace: {}\n", .{TraceString{
.action = .view_trace,
.reason = .{ .zig_error = error.DumpStackTrace },
.trace = &trace,
}});
return;
}

switch (bun.Environment.os) {
.windows => attempt_dump: {
// Windows has issues with opening the PDB file sometimes.
Expand All @@ -1371,7 +1381,7 @@ pub fn dumpStackTrace(trace: std.builtin.StackTrace) void {
var arena = bun.ArenaAllocator.init(bun.default_allocator);
defer arena.deinit();
debug.writeStackTrace(trace, stderr, arena.allocator(), debug_info, std.io.tty.detectConfig(std.io.getStdErr())) catch |err| {
stderr.print("Unable to dump stack trace: {s}\n", .{@errorName(err)}) catch return;
stderr.print("Unable to dump stack trace: {s}\nFallback trace:\n", .{@errorName(err)}) catch return;
break :attempt_dump;
};
return;
Expand All @@ -1385,21 +1395,57 @@ pub fn dumpStackTrace(trace: std.builtin.StackTrace) void {
return;
},
}
// TODO: It would be reasonable, but hacky, to spawn LLVM-symbolizer here in
// order to get the demangled stack traces.

var arena = bun.ArenaAllocator.init(bun.default_allocator);
defer arena.deinit();
var sfa = std.heap.stackFallback(16384, arena.allocator());
const alloc = sfa.get();

var argv = std.ArrayList([]const u8).init(alloc);

const program = switch (bun.Environment.os) {
.windows => "pdb-addr2line",
else => "llvm-symbolizer",
};
argv.append(program) catch return;

argv.append("--exe") catch return;
argv.append(
switch (bun.Environment.os) {
.windows => brk: {
const image_path = bun.strings.toUTF8Alloc(alloc, bun.windows.exePathW()) catch return;
break :brk std.mem.concat(alloc, u8, &.{
image_path[0 .. image_path.len - 3],
"pdb",
}) catch return;
},
else => bun.selfExePath() catch return,
},
) catch return;

var name_bytes: [1024]u8 = undefined;
for (trace.instruction_addresses[0..trace.index]) |addr| {
const line = StackLine.fromAddress(addr, &name_bytes) orelse {
stderr.print("- ??? at 0x{X}\n", .{addr}) catch break;
const line = StackLine.fromAddress(addr, &name_bytes) orelse
continue;
};
stderr.print("- {}\n", .{line}) catch break;
argv.append(std.fmt.allocPrint(alloc, "0x{X}", .{line.address}) catch return) catch return;
}
const program = switch (bun.Environment.os) {
.windows => "pdb-addr2line",
else => "llvm-symbolizer",

// std.process is used here because bun.spawnSync with libuv does not work within
// the crash handler.
const proc = std.process.Child.run(.{
.allocator = alloc,
.argv = argv.items,
}) catch {
stderr.print("Failed to invoke command: {s}\n", .{bun.fmt.fmtSlice(argv.items, " ")}) catch return;
return;
};
stderr.print("note: Use " ++ program ++ " to demangle the above trace.\n", .{}) catch return;
if (proc.term != .Exited or proc.term.Exited != 0) {
stderr.print("Failed to invoke command: {s}\n", .{bun.fmt.fmtSlice(argv.items, " ")}) catch return;
}
defer alloc.free(proc.stderr);
defer alloc.free(proc.stdout);
stderr.writeAll(proc.stdout) catch return;
stderr.writeAll(proc.stderr) catch return;
}

pub const js_bindings = struct {
Expand Down
2 changes: 1 addition & 1 deletion src/output.zig
Original file line number Diff line number Diff line change
Expand Up @@ -953,7 +953,7 @@ pub inline fn err(error_name: anytype, comptime fmt: []const u8, args: anytype)
}
}

const ScopedDebugWriter = struct {
pub const ScopedDebugWriter = struct {
pub var scoped_file_writer: File.QuietWriter = undefined;
pub threadlocal var disable_inside_log: isize = 0;
};
Expand Down
2 changes: 1 addition & 1 deletion src/sys.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1175,7 +1175,7 @@ pub fn write(fd: bun.FileDescriptor, bytes: []const u8) Maybe(usize) {
var debug_timer = bun.Output.DebugTimer.start();

defer {
if (comptime Environment.isDebug) {
if (Environment.isDebug and bun.Output.ScopedDebugWriter.disable_inside_log == 0) {
if (debug_timer.timer.read() > std.time.ns_per_ms) {
bun.Output.debugWarn("write({}, {d}) blocked for {}", .{ fd, bytes.len, debug_timer });
}
Expand Down
Loading

0 comments on commit c3157e2

Please sign in to comment.