address review

This commit is contained in:
Meghan Denny
2025-07-29 16:52:52 -07:00
parent 414c71c36d
commit 8412ef6a89
13 changed files with 49 additions and 60 deletions

View File

@@ -2061,11 +2061,10 @@ pub const Resolver = struct {
}
pub fn drainPendingCares(this: *Resolver, index: u8, err: ?c_ares.Error, timeout: i32, comptime request_type: type, comptime cares_type: type, comptime lookup_name: []const u8, result: ?*cares_type) bun.JSError!void {
const cache_name = comptime std.fmt.comptimePrint("pending_{s}_cache_cares", .{lookup_name});
this.ref();
defer this.deref();
const cache_name = comptime std.fmt.comptimePrint("pending_{s}_cache_cares", .{lookup_name});
const key = this.getKey(index, cache_name, request_type);
defer bun.default_allocator.destroy(key.lookup);
@@ -2106,12 +2105,12 @@ pub const Resolver = struct {
}
pub fn drainPendingHostCares(this: *Resolver, index: u8, err: ?c_ares.Error, timeout: i32, result: ?*c_ares.AddrInfo) bun.JSError!void {
const key = this.getKey(index, "pending_host_cache_cares", GetAddrInfoRequest);
defer bun.default_allocator.destroy(key.lookup);
this.ref();
defer this.deref();
const key = this.getKey(index, "pending_host_cache_cares", GetAddrInfoRequest);
defer bun.default_allocator.destroy(key.lookup);
var addr = result orelse {
var pending: ?*DNSLookup = key.lookup.head.next;
try key.lookup.head.processGetAddrInfo(err, timeout, null);
@@ -2151,12 +2150,12 @@ pub const Resolver = struct {
pub fn drainPendingHostNative(this: *Resolver, index: u8, globalObject: *jsc.JSGlobalObject, err: i32, result: GetAddrInfo.Result.Any) bun.JSError!void {
log("drainPendingHostNative", .{});
const key = this.getKey(index, "pending_host_cache_native", GetAddrInfoRequest);
defer bun.default_allocator.destroy(key.lookup);
this.ref();
defer this.deref();
const key = this.getKey(index, "pending_host_cache_native", GetAddrInfoRequest);
defer bun.default_allocator.destroy(key.lookup);
var array = try result.toJS(globalObject) orelse {
var pending: ?*DNSLookup = key.lookup.head.next;
var head = key.lookup.head;
@@ -2197,12 +2196,12 @@ pub const Resolver = struct {
}
pub fn drainPendingAddrCares(this: *Resolver, index: u8, err: ?c_ares.Error, timeout: i32, result: ?*c_ares.struct_hostent) bun.JSError!void {
const key = this.getKey(index, "pending_addr_cache_cares", GetHostByAddrInfoRequest);
defer bun.default_allocator.destroy(key.lookup);
this.ref();
defer this.deref();
const key = this.getKey(index, "pending_addr_cache_cares", GetHostByAddrInfoRequest);
defer bun.default_allocator.destroy(key.lookup);
var addr = result orelse {
var pending: ?*CAresReverse = key.lookup.head.next;
try key.lookup.head.processResolve(err, timeout, null);
@@ -2242,12 +2241,12 @@ pub const Resolver = struct {
}
pub fn drainPendingNameInfoCares(this: *Resolver, index: u8, err: ?c_ares.Error, timeout: i32, result: ?c_ares.struct_nameinfo) bun.JSError!void {
const key = this.getKey(index, "pending_nameinfo_cache_cares", GetNameInfoRequest);
defer bun.default_allocator.destroy(key.lookup);
this.ref();
defer this.deref();
const key = this.getKey(index, "pending_nameinfo_cache_cares", GetNameInfoRequest);
defer bun.default_allocator.destroy(key.lookup);
var name_info = result orelse {
var pending: ?*CAresNameInfo = key.lookup.head.next;
try key.lookup.head.processResolve(err, timeout, null);

View File

@@ -291,7 +291,7 @@ pub const Process = struct {
pub fn watchOrReap(this: *Process) bun.sys.Maybe(bool) {
if (this.hasExited()) {
this.onExit(this.status, &std.mem.zeroes(Rusage)) catch {}; // TODO: audit callers
this.onExit(this.status, &std.mem.zeroes(Rusage)) catch {}; // TODO: properly propagate exception upwards
return .{ .result = true };
}
@@ -299,7 +299,7 @@ pub const Process = struct {
.err => |err| {
if (comptime Environment.isPosix) {
if (err.getErrno() == .SRCH) {
this.wait(true) catch {}; // TODO: audit callers
this.wait(true) catch {}; // TODO: properly propagate exception upwards
return .{ .result = this.hasExited() };
}
}

View File

@@ -95,7 +95,7 @@ pub const ProcessHandle = struct {
.result => {},
.err => |err| {
if (!process.hasExited())
try process.onExit(.{ .err = err }, &std.mem.zeroes(bun.spawn.Rusage));
process.onExit(.{ .err = err }, &std.mem.zeroes(bun.spawn.Rusage)) catch {};
},
}
}

View File

@@ -285,7 +285,7 @@ pub const LifecycleScriptSubprocess = struct {
switch (process.watchOrReap()) {
.err => |err| {
if (!process.hasExited())
try process.onExit(.{ .err = err }, &std.mem.zeroes(bun.spawn.Rusage));
process.onExit(.{ .err = err }, &std.mem.zeroes(bun.spawn.Rusage)) catch {};
},
.result => {},
}

View File

@@ -122,7 +122,6 @@ pub fn removeReader(this: *IOReader, reader_: anytype) void {
}
}
// TODO: properly propagate exception upwards
pub fn onReadChunk(ptr: *anyopaque, chunk: []const u8, has_more: bun.io.ReadState) bool {
var this: *IOReader = @ptrCast(@alignCast(ptr));
log("IOReader(0x{x}, fd={}) onReadChunk(chunk_len={d}, has_more={s})", .{ @intFromPtr(this), this.fd, chunk.len, @tagName(has_more) });
@@ -132,7 +131,7 @@ pub fn onReadChunk(ptr: *anyopaque, chunk: []const u8, has_more: bun.io.ReadStat
while (i < this.readers.len()) {
var r = this.readers.get(i);
var remove = false;
r.onReadChunk(chunk, &remove).run() catch return false;
r.onReadChunk(chunk, &remove).run();
if (remove) {
this.readers.swapRemove(i);
} else {
@@ -159,7 +158,6 @@ pub fn onReadChunk(ptr: *anyopaque, chunk: []const u8, has_more: bun.io.ReadStat
return should_continue;
}
// TODO: properly propagate exception upwards
pub fn onReaderError(this: *IOReader, err: bun.sys.Error) void {
this.setReading(false);
this.err = err.toShellSystemError();
@@ -167,11 +165,10 @@ pub fn onReaderError(this: *IOReader, err: bun.sys.Error) void {
r.onReaderDone(if (this.err) |*e| brk: {
e.ref();
break :brk e.*;
} else null).run() catch return;
} else null).run();
}
}
// TODO: properly propagate exception upwards
pub fn onReaderDone(this: *IOReader) void {
log("IOReader(0x{x}) done", .{@intFromPtr(this)});
this.setReading(false);
@@ -179,7 +176,7 @@ pub fn onReaderDone(this: *IOReader) void {
r.onReaderDone(if (this.err) |*err| brk: {
err.ref();
break :brk err.*;
} else null).run() catch return;
} else null).run();
}
}

View File

@@ -325,7 +325,6 @@ pub fn doFileWrite(this: *IOWriter) Yield {
return this.bump(child);
}
// TODO: properly propagate exception upwards
pub fn onWritePollable(this: *IOWriter, amount: usize, status: bun.io.WriteStatus) void {
if (bun.Environment.isPosix) bun.assert(this.flags.pollable);
@@ -334,7 +333,7 @@ pub fn onWritePollable(this: *IOWriter, amount: usize, status: bun.io.WriteStatu
if (this.writer_idx >= this.writers.len()) return;
const child = this.writers.get(this.writer_idx);
if (child.isDead()) {
this.bump(child).run() catch return;
this.bump(child).run();
} else {
if (child.bytelist) |bl| {
const written_slice = this.buf.items[this.total_bytes_written .. this.total_bytes_written + amount];
@@ -367,11 +366,11 @@ pub fn onWritePollable(this: *IOWriter, amount: usize, status: bun.io.WriteStatu
// So for a quick hack we're just going to have all writes return an error.
bun.Output.debugWarn("IOWriter(0x{x}, fd={}) received done without fully writing data", .{ @intFromPtr(this), this.fd });
this.flags.broken_pipe = true;
return this.brokenPipeForWriters() catch return;
return this.brokenPipeForWriters();
}
if (child.written >= child.len) {
this.bump(child).run() catch return;
this.bump(child).run();
}
}
@@ -390,7 +389,7 @@ pub fn onWritePollable(this: *IOWriter, amount: usize, status: bun.io.WriteStatu
}
}
pub fn brokenPipeForWriters(this: *IOWriter) bun.JSExecutionTerminated!void {
pub fn brokenPipeForWriters(this: *IOWriter) void {
bun.assert(this.flags.broken_pipe);
var offset: usize = 0;
const writers = this.writers.sliceMutable()[this.writer_idx..];
@@ -401,7 +400,7 @@ pub fn brokenPipeForWriters(this: *IOWriter) bun.JSExecutionTerminated!void {
}
log("IOWriter(0x{x}, fd={}) brokenPipeForWriters Writer(0x{x}) {s}(0x{x})", .{ @intFromPtr(this), this.fd, @intFromPtr(w), @tagName(w.ptr.ptr.tag()), w.ptr.ptr.repr._ptr });
const err: jsc.SystemError = bun.sys.Error.fromCode(.PIPE, .write).toSystemError();
try w.ptr.onIOWriterChunk(0, err).run();
w.ptr.onIOWriterChunk(0, err).run();
offset += w.len;
this.cancelChunks(w.ptr);
}
@@ -443,7 +442,7 @@ pub fn onError(this: *IOWriter, err__: bun.sys.Error) void {
seen.append(@intFromPtr(ptr)) catch bun.outOfMemory();
// TODO: This probably shouldn't call .run()
w.ptr.onIOWriterChunk(0, this.err).run() catch {};
w.ptr.onIOWriterChunk(0, this.err).run();
}
this.total_bytes_written = 0;

View File

@@ -58,9 +58,6 @@ pub const Yield = union(enum) {
/// Failed and threw a JS error
failed,
done,
/// The biggest issue with the shell (which is currently not a problem since JSExecutionTerminated happens at the top of the event loop) is that this will stop execution of the shell and it will essentially leak a bunch of memory.
/// This won't be a problem when `.kill()` is implemented.
terminated,
/// Used in debug builds to ensure the shell is not creating a callstack
/// that is too deep.
@@ -74,7 +71,7 @@ pub const Yield = union(enum) {
return this.* == .done;
}
pub fn run(this: Yield) bun.JSExecutionTerminated!void {
pub fn run(this: Yield) void {
if (comptime Environment.isDebug) log("Yield({s}) _dbg_catch_exec_within_exec = {d} + 1 = {d}", .{ @tagName(this), _dbg_catch_exec_within_exec, _dbg_catch_exec_within_exec + 1 });
bun.debugAssert(_dbg_catch_exec_within_exec <= MAX_DEPTH);
if (comptime Environment.isDebug) _dbg_catch_exec_within_exec += 1;
@@ -125,7 +122,6 @@ pub const Yield = union(enum) {
}
return;
},
.terminated => return error.JSExecutionTerminated,
}
}

View File

@@ -402,7 +402,7 @@ fn parseFlag(this: *Opts, _: *Builtin, flag: []const u8) ParseFlagsResult {
return .continue_parsing;
}
pub fn onShellRmTaskDone(this: *Rm, task: *ShellRmTask) bun.JSExecutionTerminated!void {
pub fn onShellRmTaskDone(this: *Rm, task: *ShellRmTask) void {
var exec = &this.state.exec;
const tasks_done = switch (exec.state) {
.idle => @panic("Invalid state"),
@@ -430,7 +430,7 @@ pub fn onShellRmTaskDone(this: *Rm, task: *ShellRmTask) bun.JSExecutionTerminate
exec.getOutputCount(.output_done) >= exec.getOutputCount(.output_count))
{
this.state = .{ .done = .{ .exit_code = if (exec.err) |theerr| theerr.errno else 0 } };
try this.next().run();
this.next().run();
}
}

View File

@@ -88,7 +88,7 @@ fn writeNoIO(this: *@This()) Yield {
if (this.writeOnceNoIO(this.buffer[0..this.buffer_used])) |yield| return yield;
if (this.writeOnceNoIO(this.buffer[0..this.buffer_used])) |yield| return yield;
if (this.writeOnceNoIO(this.buffer[0..this.buffer_used])) |yield| return yield;
this.task.enqueue() catch return .terminated;
this.task.enqueue() catch {};
return .suspended;
}
@@ -154,12 +154,12 @@ pub const YesTask = struct {
}
}
pub fn runFromMainThread(this: *@This()) bun.JSExecutionTerminated!void {
pub fn runFromMainThread(this: *@This()) void {
const yes: *Yes = @fieldParentPtr("task", this);
return yes.writeNoIO().run();
}
pub fn runFromMainThreadMini(this: *@This(), _: *void) bun.JSExecutionTerminated!void {
pub fn runFromMainThreadMini(this: *@This(), _: *void) void {
return this.runFromMainThread();
}
};

View File

@@ -942,7 +942,7 @@ pub const Interpreter = struct {
.interp = interp,
};
interp.exit_code = exit_code;
switch (try interp.run()) {
switch (interp.run()) {
.err => |e| {
bun.Output.err(e, "Failed to run script <b>{s}<r>", .{std.fs.path.basename(path)});
bun.Global.exit(1);
@@ -1007,7 +1007,7 @@ pub const Interpreter = struct {
};
const exit_code: ExitCode = 1;
interp.exit_code = exit_code;
switch (try interp.run()) {
switch (interp.run()) {
.err => |e| {
interp.deinitEverything();
bun.Output.err(e, "Failed to run script <b>{s}<r>", .{path_for_errors});
@@ -1072,7 +1072,7 @@ pub const Interpreter = struct {
return .success;
}
pub fn run(this: *ThisInterpreter) !Maybe(void) {
pub fn run(this: *ThisInterpreter) Maybe(void) {
log("Interpreter(0x{x}) run", .{@intFromPtr(this)});
if (this.setupIOBeforeRun().asErr()) |e| {
return .{ .err = e };
@@ -1080,7 +1080,7 @@ pub const Interpreter = struct {
var root = Script.init(this, &this.root_shell, &this.args.script_ast, Script.ParentPtr.init(this), this.root_io.copy());
this.started.store(true, .seq_cst);
try root.start().run();
root.start().run();
return .success;
}
@@ -1098,7 +1098,7 @@ pub const Interpreter = struct {
var root = Script.init(this, &this.root_shell, &this.args.script_ast, Script.ParentPtr.init(this), this.root_io.copy());
this.started.store(true, .seq_cst);
try root.start().run();
root.start().run();
if (globalThis.hasException()) return error.JSError;
return .js_undefined;
@@ -1114,12 +1114,12 @@ pub const Interpreter = struct {
return buffer.toNodeBuffer(globalThis);
}
pub fn asyncCmdDone(this: *ThisInterpreter, @"async": *Async) bun.JSExecutionTerminated!void {
pub fn asyncCmdDone(this: *ThisInterpreter, @"async": *Async) void {
log("asyncCommandDone {}", .{@"async"});
@"async".actuallyDeinit();
this.async_commands_executing -= 1;
if (this.async_commands_executing == 0 and this.exit_code != null) {
try this.finish(this.exit_code.?).run();
this.finish(this.exit_code.?).run();
}
}
@@ -1152,7 +1152,7 @@ pub const Interpreter = struct {
defer loop.exit();
const stdout_js = this.getBufferedStdout(globalThis);
const stderr_js = this.getBufferedStderr(globalThis);
resolve.callMaybeEmitUncaught(globalThis, .js_undefined, &.{ .jsNumberFromU16(exit_code), stdout_js, stderr_js }) catch return .terminated;
resolve.callMaybeEmitUncaught(globalThis, .js_undefined, &.{ .jsNumberFromU16(exit_code), stdout_js, stderr_js }) catch {};
jsc.Codegen.JSShellInterpreter.resolveSetCached(this_jsvalue, globalThis, .js_undefined);
jsc.Codegen.JSShellInterpreter.rejectSetCached(this_jsvalue, globalThis, .js_undefined);
}

View File

@@ -109,7 +109,7 @@ pub fn next(this: *Async) Yield {
return .suspended;
},
.done => {
this.base.interpreter.asyncCmdDone(this) catch return .terminated;
this.base.interpreter.asyncCmdDone(this);
return .done;
},
}

View File

@@ -534,11 +534,11 @@ fn initSubproc(this: *Cmd) Yield {
if (did_exit_immediately) {
if (subproc.process.hasExited()) {
// process has already exited, we called wait4(), but we did not call onProcessExit()
subproc.process.onExit(subproc.process.status, &std.mem.zeroes(bun.spawn.Rusage)) catch return .terminated;
subproc.process.onExit(subproc.process.status, &std.mem.zeroes(bun.spawn.Rusage)) catch {};
} else {
// process has already exited, but we haven't called wait4() yet
// https://cs.github.com/libuv/libuv/blob/b00d1bd225b602570baee82a6152eaa823a84fa6/src/unix/process.c#L1007
subproc.process.wait(false) catch return .terminated;
subproc.process.wait(false) catch {};
}
}
@@ -676,14 +676,14 @@ pub fn hasFinished(this: *Cmd) bool {
}
/// Called by Subprocess
pub fn onExit(this: *Cmd, exit_code: ExitCode) bun.JSExecutionTerminated!void {
pub fn onExit(this: *Cmd, exit_code: ExitCode) void {
this.exit_code = exit_code;
const has_finished = this.hasFinished();
log("cmd exit code={d} has_finished={any} ({x})", .{ exit_code, has_finished, @intFromPtr(this) });
if (has_finished) {
this.state = .done;
try this.next().run();
this.next().run();
}
}

View File

@@ -903,7 +903,7 @@ pub const ShellSubprocess = struct {
return this.process.wait(sync);
}
pub fn onProcessExit(this: *@This(), _: *Process, status: bun.spawn.Status, _: *const bun.spawn.Rusage) bun.JSExecutionTerminated!void {
pub fn onProcessExit(this: *@This(), _: *Process, status: bun.spawn.Status, _: *const bun.spawn.Rusage) void {
log("onProcessExit({x}, {any})", .{ @intFromPtr(this), status });
const exit_code: ?u8 = brk: {
if (status == .exited) {
@@ -926,7 +926,7 @@ pub const ShellSubprocess = struct {
if (exit_code) |code| {
const cmd = this.cmd_parent;
if (cmd.exit_code == null) {
try cmd.onExit(code);
cmd.onExit(code);
}
}
}
@@ -1173,7 +1173,6 @@ pub const PipeReader = struct {
return should_continue;
}
// TODO: properly propagate exception upwards
pub fn onReaderDone(this: *PipeReader) void {
log("onReaderDone(0x{x}, {s})", .{ @intFromPtr(this), @tagName(this.out_type) });
const owned = this.toOwnedSlice();
@@ -1184,7 +1183,7 @@ pub const PipeReader = struct {
defer this.deref();
defer if (this.process) |_| this.deref();
defer if (this.process) |p| p.onCloseIO(this.kind(p));
this.trySignalDoneToCmd().run() catch return;
this.trySignalDoneToCmd().run();
}
pub fn trySignalDoneToCmd(
@@ -1292,7 +1291,6 @@ pub const PipeReader = struct {
}
}
// TODO: properly propagate exception upwards
pub fn onReaderError(this: *PipeReader, err: bun.sys.Error) void {
log("PipeReader(0x{x}) onReaderError {}", .{ @intFromPtr(this), err });
if (this.state == .done) {
@@ -1304,7 +1302,7 @@ pub const PipeReader = struct {
defer this.deref();
defer if (this.process) |_| this.deref();
defer if (this.process) |p| p.onCloseIO(this.kind(p));
this.trySignalDoneToCmd().run() catch return;
this.trySignalDoneToCmd().run();
}
pub fn close(this: *PipeReader) void {