Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(windows spawn): use Job Object to manage subprocesses of subprocesses #11240

Merged
merged 7 commits into from
May 24, 2024
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
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 @@ -999,7 +999,7 @@ const StackLine = struct {
};

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

Expand Down Expand Up @@ -1351,6 +1351,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 @@ -1361,7 +1371,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 @@ -1375,21 +1385,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
Loading