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 5 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
2 changes: 1 addition & 1 deletion 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
98 changes: 75 additions & 23 deletions src/bun.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2232,8 +2232,33 @@ 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| {
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 +2284,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 +2344,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 +2377,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
64 changes: 64 additions & 0 deletions src/windows.zig
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ pub const FILE_BEGIN = windows.FILE_BEGIN;
pub const FILE_END = windows.FILE_END;
pub const FILE_CURRENT = windows.FILE_CURRENT;
pub const ULONG = windows.ULONG;
pub const ULONGLONG = windows.ULONGLONG;
pub const UINT = windows.UINT;
pub const LARGE_INTEGER = windows.LARGE_INTEGER;
pub const UNICODE_STRING = windows.UNICODE_STRING;
Expand Down Expand Up @@ -3097,7 +3098,39 @@ pub const JOBOBJECT_ASSOCIATE_COMPLETION_PORT = extern struct {
CompletionPort: HANDLE,
};

pub const JOBOBJECT_EXTENDED_LIMIT_INFORMATION = extern struct {
BasicLimitInformation: JOBOBJECT_BASIC_LIMIT_INFORMATION,
///Reserved
IoInfo: IO_COUNTERS,
ProcessMemoryLimit: usize,
JobMemoryLimit: usize,
PeakProcessMemoryUsed: usize,
PeakJobMemoryUsed: usize,
};

pub const IO_COUNTERS = extern struct {
ReadOperationCount: ULONGLONG,
WriteOperationCount: ULONGLONG,
OtherOperationCount: ULONGLONG,
ReadTransferCount: ULONGLONG,
WriteTransferCount: ULONGLONG,
OtherTransferCount: ULONGLONG,
};

pub const JOBOBJECT_BASIC_LIMIT_INFORMATION = extern struct {
PerProcessUserTimeLimit: LARGE_INTEGER,
PerJobUserTimeLimit: LARGE_INTEGER,
LimitFlags: DWORD,
MinimumWorkingSetSize: usize,
MaximumWorkingSetSize: usize,
ActiveProcessLimit: DWORD,
Affinity: *ULONG,
PriorityClass: DWORD,
SchedulingClass: DWORD,
};

pub const JobObjectAssociateCompletionPortInformation: DWORD = 7;
pub const JobObjectExtendedLimitInformation: DWORD = 9;

pub extern "kernel32" fn SetInformationJobObject(
hJob: HANDLE,
Expand Down Expand Up @@ -3528,3 +3561,34 @@ pub fn DeleteFileBun(sub_path_w: []const u16, options: DeleteFileOptions) bun.JS

pub const EXCEPTION_CONTINUE_EXECUTION = -1;
pub const MS_VC_EXCEPTION = 0x406d1388;

pub const STARTUPINFOEXW = extern struct {
StartupInfo: std.os.windows.STARTUPINFOW,
lpAttributeList: [*]u8,
};

pub extern "kernel32" fn InitializeProcThreadAttributeList(
lpAttributeList: ?[*]u8,
dwAttributeCount: DWORD,
dwFlags: DWORD,
size: *usize,
) BOOL;

pub extern "kernel32" fn UpdateProcThreadAttribute(
lpAttributeList: [*]u8, // [in, out]
dwFlags: DWORD, // [in]
Attribute: windows.DWORD_PTR, // [in]
lpValue: *const anyopaque, // [in]
cbSize: usize, // [in]
lpPreviousValue: ?*anyopaque, // [out, optional]
lpReturnSize: ?*usize, // [in, optional]
) BOOL;

pub extern "kernel32" fn IsProcessInJob(process: HANDLE, job: HANDLE, result: *BOOL) BOOL;

pub const EXTENDED_STARTUPINFO_PRESENT = 0x80000;
pub const PROC_THREAD_ATTRIBUTE_JOB_LIST = 0x2000D;
pub const JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE = 0x2000;
pub const JOB_OBJECT_LIMIT_DIE_ON_UNHANDLED_EXCEPTION = 0x400;
pub const JOB_OBJECT_LIMIT_BREAKAWAY_OK = 0x800;
pub const JOB_OBJECT_LIMIT_SILENT_BREAKAWAY_OK = 0x00001000;
1 change: 1 addition & 0 deletions test/js/bun/spawn/empty.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
// empty file
14 changes: 14 additions & 0 deletions test/js/bun/spawn/job-object-bug.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { spawn } from "bun";
import { it, expect } from "bun:test";
import { bunEnv, bunExe } from "harness";
import { join } from "node:path";

it("does not hang", async () => {
const subprocess = spawn({
cmd: [bunExe(), "test", join(import.meta.dirname, "job-object-bug.ts")],
env: bunEnv,
stdio: ["ignore", "pipe", "pipe"],
});
await Bun.readableStreamToText(subprocess.stdout);
expect(await subprocess.exited).toBe(0);
});
16 changes: 16 additions & 0 deletions test/js/bun/spawn/job-object-bug.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { spawn } from "bun";
import { bunEnv, bunExe } from "harness";
import { join } from "node:path";

// prior, this would hang on Windows if you ran this with a pipe

const run = spawn({
cmd: [bunExe(), "--watch", join(import.meta.dirname, "empty.js")],
stdout: "inherit",
stderr: "inherit",
stdin: "ignore",
env: bunEnv,
});
await Bun.sleep(250);
run.kill(9);
await run.exited;
Loading