From 081c7fff00fa48af9c558270779306855edc0d20 Mon Sep 17 00:00:00 2001 From: Ashcon Partovi Date: Wed, 27 Mar 2024 18:28:48 -0700 Subject: [PATCH 1/5] Add missing aliases to `expect()` types --- packages/bun-types/test.d.ts | 46 ++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/packages/bun-types/test.d.ts b/packages/bun-types/test.d.ts index da522ea9ec..8ec751d42a 100644 --- a/packages/bun-types/test.d.ts +++ b/packages/bun-types/test.d.ts @@ -1127,6 +1127,27 @@ declare module "bun:test" { * @param expected the expected error, error message, or error pattern */ toThrow(expected?: unknown): void; + /** + * Asserts that a function throws an error. + * + * - If expected is a `string` or `RegExp`, it will check the `message` property. + * - If expected is an `Error` object, it will check the `name` and `message` properties. + * - If expected is an `Error` constructor, it will check the class of the `Error`. + * - If expected is not provided, it will check if anything as thrown. + * + * @example + * function fail() { + * throw new Error("Oops!"); + * } + * expect(fail).toThrowError("Oops!"); + * expect(fail).toThrowError(/oops/i); + * expect(fail).toThrowError(Error); + * expect(fail).toThrowError(); + * + * @param expected the expected error, error message, or error pattern + * @alias toThrow + */ + toThrowError(expected?: unknown): void; /** * Asserts that a value matches a regular expression or includes a substring. * @@ -1410,22 +1431,47 @@ declare module "bun:test" { * Ensures that a mock function is called. */ toHaveBeenCalled(): void; + /** + * Ensures that a mock function is called an exact number of times. + * @alias toHaveBeenCalled + */ + toBeCalled(): void; /** * Ensures that a mock function is called an exact number of times. */ toHaveBeenCalledTimes(expected: number): void; + /** + * Ensure that a mock function is called with specific arguments. + * @alias toHaveBeenCalledTimes + */ + toBeCalledTimes(expected: number): void; /** * Ensure that a mock function is called with specific arguments. */ toHaveBeenCalledWith(...expected: unknown[]): void; + /** + * Ensure that a mock function is called with specific arguments. + * @alias toHaveBeenCalledWith + */ + toBeCalledWith(...expected: unknown[]): void; /** * Ensure that a mock function is called with specific arguments for the last call. */ toHaveBeenLastCalledWith(...expected: unknown[]): void; + /** + * Ensure that a mock function is called with specific arguments for the nth call. + * @alias toHaveBeenCalledWith + */ + lastCalledWith(...expected: unknown[]): void; /** * Ensure that a mock function is called with specific arguments for the nth call. */ toHaveBeenNthCalledWith(n: number, ...expected: unknown[]): void; + /** + * Ensure that a mock function is called with specific arguments for the nth call. + * @alias toHaveBeenCalledWith + */ + nthCalledWith(n: number, ...expected: unknown[]): void; } /** From 79ced2767a5ff33352e4d6372a32b9308fa281e2 Mon Sep 17 00:00:00 2001 From: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com> Date: Wed, 27 Mar 2024 19:16:23 -0700 Subject: [PATCH 2/5] Let's try calling close_range CLOSE_RANGE_CLOEXEC starting on 4 instead of 0 --- src/bun.js/bindings/c-bindings.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bun.js/bindings/c-bindings.cpp b/src/bun.js/bindings/c-bindings.cpp index f77f9e6bb5..6526c9eb31 100644 --- a/src/bun.js/bindings/c-bindings.cpp +++ b/src/bun.js/bindings/c-bindings.cpp @@ -409,7 +409,7 @@ extern "C" void bun_initialize_process() // This is less of an issue for macOS due to posix_spawn // This is best effort, not all linux kernels support close_range or CLOSE_RANGE_CLOEXEC // To avoid breaking --watch, we skip stdin, stdout, stderr and IPC. - bun_close_range(0, ~0U, CLOSE_RANGE_CLOEXEC); + bun_close_range(4, ~0U, CLOSE_RANGE_CLOEXEC); #endif #if OS(LINUX) || OS(DARWIN) From db1283e982be363acfef689a670a5bd94f2888dc Mon Sep 17 00:00:00 2001 From: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com> Date: Wed, 27 Mar 2024 20:09:31 -0700 Subject: [PATCH 3/5] Fixes #9669 --- src/bun.js/bindings/c-bindings.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/bun.js/bindings/c-bindings.cpp b/src/bun.js/bindings/c-bindings.cpp index 6526c9eb31..3764000024 100644 --- a/src/bun.js/bindings/c-bindings.cpp +++ b/src/bun.js/bindings/c-bindings.cpp @@ -194,7 +194,12 @@ extern "C" void windows_enable_stdio_inheritance() // close_range is glibc > 2.33, which is very new extern "C" ssize_t bun_close_range(unsigned int start, unsigned int end, unsigned int flags) { +// https://github.com/oven-sh/bun/issues/9669 +#ifdef __NR_close_range return syscall(__NR_close_range, start, end, flags); +#else + return ENOSYS; +#endif } static void unset_cloexec(int fd) From d66ace959d94b40af49217daa653064a3a800061 Mon Sep 17 00:00:00 2001 From: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com> Date: Wed, 27 Mar 2024 20:27:16 -0700 Subject: [PATCH 4/5] Use `&= ~` instead of `^=` --- src/bun.js/bindings/bun-spawn.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bun.js/bindings/bun-spawn.cpp b/src/bun.js/bindings/bun-spawn.cpp index 8014dfcabc..18a6c59ed7 100644 --- a/src/bun.js/bindings/bun-spawn.cpp +++ b/src/bun.js/bindings/bun-spawn.cpp @@ -121,7 +121,7 @@ extern "C" ssize_t posix_spawn_bun( // Remove the O_CLOEXEC flag // If we don't do this, then the process will have an already-closed file descriptor int mask = fcntl(action.fds[0], F_GETFD, 0); - mask ^= FD_CLOEXEC; + mask &= ~FD_CLOEXEC; fcntl(action.fds[0], F_SETFD, mask); if (errno != 0) { From dea877f19b3a271c3bd8c062604f18d7f8927fef Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Wed, 27 Mar 2024 20:56:54 -0700 Subject: [PATCH 5/5] Revert "Fix some shell regressions (#9630)" (#9670) This reverts commit aee8eeaf45e7c173409737f2a48774c409ea6cd0. --- src/bun.js/event_loop.zig | 6 - src/shell/interpreter.zig | 309 +++++++++-------------------- src/shell/shell.zig | 5 - test/js/bun/shell/bunshell.test.ts | 76 +------ test/js/bun/shell/leak.test.ts | 5 +- 5 files changed, 99 insertions(+), 302 deletions(-) diff --git a/src/bun.js/event_loop.zig b/src/bun.js/event_loop.zig index cf618e3172..b6a164e13b 100644 --- a/src/bun.js/event_loop.zig +++ b/src/bun.js/event_loop.zig @@ -363,7 +363,6 @@ const ShellIOWriterAsyncDeinit = bun.shell.Interpreter.AsyncDeinitWriter; const TimerReference = JSC.BunTimer.Timeout.TimerReference; const ProcessWaiterThreadTask = if (Environment.isPosix) bun.spawn.WaiterThread.ProcessQueue.ResultTask else opaque {}; const ProcessMiniEventLoopWaiterThreadTask = if (Environment.isPosix) bun.spawn.WaiterThread.ProcessMiniEventLoopQueue.ResultTask else opaque {}; -const ShellAsyncSubprocessDone = bun.shell.Interpreter.Cmd.ShellAsyncSubprocessDone; const RuntimeTranspilerStore = JSC.RuntimeTranspilerStore; // Task.get(ReadFileTask) -> ?ReadFileTask pub const Task = TaggedPointerUnion(.{ @@ -433,7 +432,6 @@ pub const Task = TaggedPointerUnion(.{ ShellLsTask, ShellMkdirTask, ShellTouchTask, - ShellAsyncSubprocessDone, TimerReference, ProcessWaiterThreadTask, @@ -885,10 +883,6 @@ pub const EventLoop = struct { while (@field(this, queue_name).readItem()) |task| { defer counter += 1; switch (task.tag()) { - @field(Task.Tag, typeBaseName(@typeName(ShellAsyncSubprocessDone))) => { - var shell_ls_task: *ShellAsyncSubprocessDone = task.get(ShellAsyncSubprocessDone).?; - shell_ls_task.runFromMainThread(); - }, @field(Task.Tag, typeBaseName(@typeName(ShellIOWriterAsyncDeinit))) => { var shell_ls_task: *ShellIOWriterAsyncDeinit = task.get(ShellIOWriterAsyncDeinit).?; shell_ls_task.runFromMainThread(); diff --git a/src/shell/interpreter.zig b/src/shell/interpreter.zig index 42841c6312..667e46f742 100644 --- a/src/shell/interpreter.zig +++ b/src/shell/interpreter.zig @@ -605,11 +605,7 @@ pub const Interpreter = struct { has_pending_activity: std.atomic.Value(usize) = std.atomic.Value(usize).init(0), started: std.atomic.Value(bool) = std.atomic.Value(bool).init(false), - flags: packed struct(u8) { - done: bool = false, - quiet: bool = false, - __unused: u6 = 0, - } = .{}, + done: ?*bool = null, exit_code: ?*ExitCode = null, const InterpreterChildPtr = StatePtrUnion(.{ @@ -1082,7 +1078,29 @@ pub const Interpreter = struct { .err => |err| return .{ .err = .{ .sys = err.toSystemError() } }, }; + log("Duping stdout", .{}); + const stdout_fd = switch (ShellSyscall.dup(shell.STDOUT_FD)) { + .result => |fd| fd, + .err => |err| return .{ .err = .{ .sys = err.toSystemError() } }, + }; + + log("Duping stderr", .{}); + const stderr_fd = switch (ShellSyscall.dup(shell.STDERR_FD)) { + .result => |fd| fd, + .err => |err| return .{ .err = .{ .sys = err.toSystemError() } }, + }; + const stdin_reader = IOReader.init(stdin_fd, event_loop); + const stdout_writer = IOWriter.init( + stdout_fd, + .{ + .pollable = isPollable(stdout_fd, event_loop.stdout().data.file.mode), + }, + event_loop, + ); + const stderr_writer = IOWriter.init(stderr_fd, .{ + .pollable = isPollable(stderr_fd, event_loop.stderr().data.file.mode), + }, event_loop); interpreter.* = .{ .event_loop = event_loop, @@ -1107,15 +1125,24 @@ pub const Interpreter = struct { .stdin = .{ .fd = stdin_reader, }, - // By default stdout/stderr should be an IOWriter writing to a dup'ed stdout/stderr - // But if the user later calls `.setQuiet(true)` then all those syscalls/initialization was pointless work - // So we cheaply initialize them now as `.pipe` - // When `Interpreter.run()` is called, we check if `this.flags.quiet == false`, if so then we then properly initialize the IOWriter - .stdout = .pipe, - .stderr = .pipe, + .stdout = .{ + .fd = .{ + .writer = stdout_writer, + }, + }, + .stderr = .{ + .fd = .{ + .writer = stderr_writer, + }, + }, }, }; + if (event_loop == .js) { + interpreter.root_io.stdout.fd.captured = &interpreter.root_shell._buffered_stdout.owned; + interpreter.root_io.stderr.fd.captured = &interpreter.root_shell._buffered_stderr.owned; + } + return .{ .result = interpreter }; } @@ -1166,26 +1193,17 @@ pub const Interpreter = struct { var exit_code: ExitCode = 1; const IsDone = struct { - interp: *const Interpreter, + done: bool = false, fn isDone(this: *anyopaque) bool { const asdlfk = bun.cast(*const @This(), this); - return asdlfk.interp.flags.done; + return asdlfk.done; } }; - var is_done: IsDone = .{ - .interp = interp, - }; + var is_done: IsDone = .{}; + interp.done = &is_done.done; interp.exit_code = &exit_code; - switch (try interp.run()) { - .err => |e| { - interp.deinitEverything(); - bun.Output.prettyErrorln("error: Failed to run script {s} due to error {}", .{ std.fs.path.basename(path), e.toSystemError() }); - bun.Global.exit(1); - return 1; - }, - else => {}, - } + try interp.run(); mini.tick(&is_done, @as(fn (*anyopaque) bool, IsDone.isDone)); return exit_code; } @@ -1215,7 +1233,7 @@ pub const Interpreter = struct { }; const script_heap = try arena.allocator().create(ast.Script); script_heap.* = script; - var interp: *ThisInterpreter = switch (ThisInterpreter.init(.{ .mini = mini }, bun.default_allocator, &arena, script_heap, jsobjs)) { + var interp = switch (ThisInterpreter.init(.{ .mini = mini }, bun.default_allocator, &arena, script_heap, jsobjs)) { .err => |*e| { throwShellErr(e, .{ .mini = mini }); return 1; @@ -1223,102 +1241,33 @@ pub const Interpreter = struct { .result => |i| i, }; const IsDone = struct { - interp: *const Interpreter, + done: bool = false, fn isDone(this: *anyopaque) bool { const asdlfk = bun.cast(*const @This(), this); - return asdlfk.interp.flags.done; + return asdlfk.done; } }; - var is_done: IsDone = .{ - .interp = interp, - }; + var is_done: IsDone = .{}; var exit_code: ExitCode = 1; + interp.done = &is_done.done; interp.exit_code = &exit_code; - switch (try interp.run()) { - .err => |e| { - defer interp.deinitEverything(); - bun.Output.prettyErrorln("error: Failed to run script {s} due to error {}", .{ path_for_errors, e.toSystemError() }); - bun.Global.exit(1); - return 1; - }, - else => {}, - } + try interp.run(); mini.tick(&is_done, @as(fn (*anyopaque) bool, IsDone.isDone)); interp.deinitEverything(); return exit_code; } - fn setupIOBeforeRun(this: *ThisInterpreter) Maybe(void) { - if (!this.flags.quiet) { - const event_loop = this.event_loop; - - log("Duping stdout", .{}); - const stdout_fd = switch (ShellSyscall.dup(shell.STDOUT_FD)) { - .result => |fd| fd, - .err => |err| return .{ .err = err }, - }; - - log("Duping stderr", .{}); - const stderr_fd = switch (ShellSyscall.dup(shell.STDERR_FD)) { - .result => |fd| fd, - .err => |err| return .{ .err = err }, - }; - - const stdout_writer = IOWriter.init( - stdout_fd, - .{ - .pollable = isPollable(stdout_fd, event_loop.stdout().data.file.mode), - }, - event_loop, - ); - const stderr_writer = IOWriter.init(stderr_fd, .{ - .pollable = isPollable(stderr_fd, event_loop.stderr().data.file.mode), - }, event_loop); - - this.root_io = .{ - .stdin = this.root_io.stdin, - .stdout = .{ - .fd = .{ - .writer = stdout_writer, - }, - }, - .stderr = .{ - .fd = .{ - .writer = stderr_writer, - }, - }, - }; - - if (event_loop == .js) { - this.root_io.stdout.fd.captured = &this.root_shell._buffered_stdout.owned; - this.root_io.stderr.fd.captured = &this.root_shell._buffered_stderr.owned; - } - } - - return Maybe(void).success; - } - - pub fn run(this: *ThisInterpreter) !Maybe(void) { - if (this.setupIOBeforeRun().asErr()) |e| { - return .{ .err = e }; - } + pub fn run(this: *ThisInterpreter) !void { var root = Script.init(this, &this.root_shell, this.script, Script.ParentPtr.init(this), this.root_io.copy()); this.started.store(true, .SeqCst); root.start(); - - return Maybe(void).success; } pub fn runFromJS(this: *ThisInterpreter, globalThis: *JSGlobalObject, callframe: *JSC.CallFrame) callconv(.C) JSValue { _ = callframe; // autofix - if (this.setupIOBeforeRun().asErr()) |e| { - defer this.deinitEverything(); - const shellerr = bun.shell.ShellErr.newSys(e); - throwShellErr(&shellerr, .{ .js = globalThis.bunVM().event_loop }); - return .undefined; - } + _ = globalThis; incrPendingActivityFlag(&this.has_pending_activity); var root = Script.init(this, &this.root_shell, this.script, Script.ParentPtr.init(this), this.root_io.copy()); this.started.store(true, .SeqCst); @@ -1358,7 +1307,7 @@ pub const Interpreter = struct { defer this.deinitAfterJSRun(); _ = this.resolve.call(&.{JSValue.jsNumberFromU16(exit_code)}); } else { - this.flags.done = true; + this.done.?.* = true; this.exit_code.?.* = exit_code; } } @@ -1426,9 +1375,14 @@ pub const Interpreter = struct { return .undefined; } - pub fn setQuiet(this: *ThisInterpreter, _: *JSGlobalObject, _: *JSC.CallFrame) callconv(.C) JSC.JSValue { + pub fn setQuiet(this: *ThisInterpreter, globalThis: *JSGlobalObject, callframe: *JSC.CallFrame) callconv(.C) JSC.JSValue { log("Interpreter(0x{x}) setQuiet()", .{@intFromPtr(this)}); - this.flags.quiet = true; + _ = globalThis; + _ = callframe; + this.root_io.stdout.deref(); + this.root_io.stderr.deref(); + this.root_io.stdout = .pipe; + this.root_io.stderr = .pipe; return .undefined; } @@ -3087,14 +3041,10 @@ pub const Interpreter = struct { fn writePipe(pipes: []Pipe, proc_idx: usize, cmd_count: usize, io: *IO, evtloop: JSC.EventLoopHandle) IO.OutKind { // Last command in the pipeline should write to stdout if (proc_idx == cmd_count - 1) return io.stdout.ref(); - return .{ - .fd = .{ - .writer = IOWriter.init(pipes[proc_idx][1], .{ - .pollable = true, - .is_socket = bun.Environment.isPosix, - }, evtloop), - }, - }; + return .{ .fd = .{ .writer = IOWriter.init(pipes[proc_idx][1], .{ + .pollable = true, + .is_socket = bun.Environment.isPosix, + }, evtloop) } }; } fn readPipe(pipes: []Pipe, proc_idx: usize, io: *IO, evtloop: JSC.EventLoopHandle) IO.InKind { @@ -3148,48 +3098,6 @@ pub const Interpreter = struct { waiting_write_err, }, - /// If a subprocess and its stdout/stderr exit immediately, we queue - /// completion of this `Cmd` onto the event loop to avoid having the Cmd - /// unexpectedly deinitalizing deeper in the callstack and becoming - /// undefined memory. - pub const ShellAsyncSubprocessDone = struct { - cmd: *Cmd, - concurrent_task: JSC.EventLoopTask, - - pub fn format(this: *const ShellAsyncSubprocessDone, comptime fmt: []const u8, opts: std.fmt.FormatOptions, writer: anytype) !void { - _ = fmt; // autofix - _ = opts; // autofix - try writer.print("ShellAsyncSubprocessDone(0x{x}, cmd=0{x})", .{ @intFromPtr(this), @intFromPtr(this.cmd) }); - } - - pub fn enqueue(this: *ShellAsyncSubprocessDone) void { - log("{} enqueue", .{this}); - const ctx = this; - const evtloop = this.cmd.base.eventLoop(); - - if (evtloop == .js) { - evtloop.js.enqueueTaskConcurrent(this.concurrent_task.js.from(ctx, .manual_deinit)); - } else { - evtloop.mini.enqueueTaskConcurrent(this.concurrent_task.mini.from(ctx, "runFromMainThreadMini")); - } - } - - pub fn runFromMainThreadMini(this: *@This(), _: *void) void { - this.runFromMainThread(); - } - - pub fn runFromMainThread(this: *ShellAsyncSubprocessDone) void { - log("{} runFromMainThread", .{this}); - defer this.deinit(); - this.cmd.parent.childDone(this.cmd, this.cmd.exit_code orelse 0); - } - - pub fn deinit(this: *ShellAsyncSubprocessDone) void { - log("{} deinit", .{this}); - bun.destroy(this); - } - }; - const Subprocess = bun.shell.subproc.ShellSubprocess; pub const Exec = union(enum) { @@ -3822,15 +3730,7 @@ pub const Interpreter = struct { .stderr => this.bufferedOutputCloseStderr(err), } if (this.hasFinished()) { - if (!this.spawn_arena_freed) { - var async_subprocess_done = bun.new(ShellAsyncSubprocessDone, .{ - .cmd = this, - .concurrent_task = JSC.EventLoopTask.fromEventLoop(this.base.eventLoop()), - }); - async_subprocess_done.enqueue(); - } else { - this.parent.childDone(this, this.exit_code orelse 0); - } + this.parent.childDone(this, this.exit_code orelse 0); } } @@ -4476,7 +4376,6 @@ pub const Interpreter = struct { }; } - /// **WARNING** You should make sure that stdout/stderr does not need IO (e.g. `.needsIO(.stderr)` is false before caling `.writeNoIO(.stderr, buf)`) pub fn writeNoIO(this: *Builtin, comptime io_kind: @Type(.EnumLiteral), buf: []const u8) usize { if (comptime io_kind != .stdout and io_kind != .stderr) { @compileError("Bad IO" ++ @tagName(io_kind)); @@ -4487,7 +4386,7 @@ pub const Interpreter = struct { var io: *BuiltinIO.Output = &@field(this, @tagName(io_kind)); switch (io.*) { - .fd => @panic("writeNoIO(. " ++ @tagName(io_kind) ++ ", buf) can't write to a file descriptor, did you check that needsIO(." ++ @tagName(io_kind) ++ ") was false?"), + .fd => @panic("writeNoIO can't write to a file descriptor"), .buf => { log("{s} write to buf len={d} str={s}{s}\n", .{ this.kind.asString(), buf.len, buf[0..@min(buf.len, 16)], if (buf.len > 16) "..." else "" }); io.buf.appendSlice(buf) catch bun.outOfMemory(); @@ -4911,14 +4810,8 @@ pub const Interpreter = struct { done, } = .idle, - pub fn format(this: *const Touch, comptime fmt: []const u8, opts: std.fmt.FormatOptions, writer: anytype) !void { - _ = fmt; // autofix - _ = opts; // autofix - try writer.print("Touch(0x{x}, state={s})", .{ @intFromPtr(this), @tagName(this.state) }); - } - pub fn deinit(this: *Touch) void { - log("{} deinit", .{this}); + _ = this; } pub fn start(this: *Touch) Maybe(void) { @@ -5003,8 +4896,6 @@ pub const Interpreter = struct { } pub fn onShellTouchTaskDone(this: *Touch, task: *ShellTouchTask) void { - log("{} onShellTouchTaskDone {} tasks_done={d} tasks_count={d}", .{ this, task, this.state.exec.tasks_done, this.state.exec.tasks_count }); - defer bun.default_allocator.destroy(task); this.state.exec.tasks_done += 1; const err = task.err; @@ -5080,12 +4971,6 @@ pub const Interpreter = struct { event_loop: JSC.EventLoopHandle, concurrent_task: JSC.EventLoopTask, - pub fn format(this: *const ShellTouchTask, comptime fmt: []const u8, opts: std.fmt.FormatOptions, writer: anytype) !void { - _ = fmt; // autofix - _ = opts; // autofix - try writer.print("ShellTouchTask(0x{x}, filepath={s})", .{ @intFromPtr(this), this.filepath }); - } - const print = bun.Output.scoped(.ShellTouchTask, false); pub fn deinit(this: *ShellTouchTask) void { @@ -5109,12 +4994,12 @@ pub const Interpreter = struct { } pub fn schedule(this: *@This()) void { - print("{} schedule", .{this}); + print("schedule", .{}); WorkPool.schedule(&this.task); } pub fn runFromMainThread(this: *@This()) void { - print("{} runFromJS", .{this}); + print("runFromJS", .{}); this.touch.onShellTouchTaskDone(this); } @@ -5124,7 +5009,6 @@ pub const Interpreter = struct { fn runFromThreadPool(task: *JSC.WorkPoolTask) void { var this: *ShellTouchTask = @fieldParentPtr(ShellTouchTask, "task", task); - print("{} runFromThreadPool", .{this}); // We have to give an absolute path const filepath: [:0]const u8 = brk: { @@ -5478,12 +5362,6 @@ pub const Interpreter = struct { return out; } - pub fn format(this: *const ShellMkdirTask, comptime fmt_: []const u8, options_: std.fmt.FormatOptions, writer: anytype) !void { - _ = fmt_; // autofix - _ = options_; // autofix - try writer.print("ShellMkdirTask(0x{x}, filepath={s})", .{ @intFromPtr(this), this.filepath }); - } - pub fn create( mkdir: *Mkdir, opts: Opts, @@ -5505,12 +5383,12 @@ pub const Interpreter = struct { } pub fn schedule(this: *@This()) void { - print("{} schedule", .{this}); + print("schedule", .{}); WorkPool.schedule(&this.task); } pub fn runFromMainThread(this: *@This()) void { - print("{} runFromJS", .{this}); + print("runFromJS", .{}); this.mkdir.onShellMkdirTaskDone(this); } @@ -5520,7 +5398,6 @@ pub const Interpreter = struct { fn runFromThreadPool(task: *JSC.WorkPoolTask) void { var this: *ShellMkdirTask = @fieldParentPtr(ShellMkdirTask, "task", task); - print("{} runFromThreadPool", .{this}); // We have to give an absolute path to our mkdir // implementation for it to work with cwd @@ -8823,7 +8700,7 @@ pub const Interpreter = struct { pub fn asyncDeinit(this: *@This()) void { log("IOReader(0x{x}) asyncDeinit", .{@intFromPtr(this)}); - this.async_deinit.enqueue(); + this.async_deinit.schedule(); } pub fn __deinit(this: *@This()) void { @@ -8852,14 +8729,11 @@ pub const Interpreter = struct { }; pub const AsyncDeinitWriter = struct { - ran: bool = false, - - pub fn enqueue(this: *@This()) void { - if (this.ran) return; - this.ran = true; + task: WorkPoolTask = .{ .callback = &runFromThreadPool }, + pub fn runFromThreadPool(task: *WorkPoolTask) void { + var this = @fieldParentPtr(@This(), "task", task); var iowriter = this.writer(); - if (iowriter.evtloop == .js) { iowriter.evtloop.js.enqueueTaskConcurrent(iowriter.concurrent_task.js.from(this, .manual_deinit)); } else { @@ -8879,15 +8753,17 @@ pub const Interpreter = struct { pub fn runFromMainThreadMini(this: *@This(), _: *void) void { this.runFromMainThread(); } + + pub fn schedule(this: *@This()) void { + WorkPool.schedule(&this.task); + } }; pub const AsyncDeinit = struct { - ran: bool = false, - - pub fn enqueue(this: *@This()) void { - if (this.ran) return; - this.ran = true; + task: WorkPoolTask = .{ .callback = &runFromThreadPool }, + pub fn runFromThreadPool(task: *WorkPoolTask) void { + var this = @fieldParentPtr(AsyncDeinit, "task", task); var ioreader = this.reader(); if (ioreader.evtloop == .js) { ioreader.evtloop.js.enqueueTaskConcurrent(ioreader.concurrent_task.js.from(this, .manual_deinit)); @@ -8908,6 +8784,10 @@ pub const Interpreter = struct { pub fn runFromMainThreadMini(this: *AsyncDeinit, _: *void) void { this.runFromMainThread(); } + + pub fn schedule(this: *AsyncDeinit) void { + WorkPool.schedule(&this.task); + } }; pub const IOWriter = struct { @@ -9245,9 +9125,6 @@ pub const Interpreter = struct { if (is_dead) { this.skipDead(); } else { - if (bun.Environment.allow_assert) { - if (!is_dead) std.debug.assert(current_writer.written == current_writer.len); - } this.__idx += 1; } @@ -9262,15 +9139,15 @@ pub const Interpreter = struct { if (this.total_bytes_written >= SHRINK_THRESHOLD) { log("IOWriter(0x{x}, fd={}) exceeded shrink threshold: truncating", .{ @intFromPtr(this), this.fd }); - const slice = this.buf.items[this.total_bytes_written..]; - const remaining_len = slice.len; - if (slice.len == 0) { + const remaining_len = this.total_bytes_written - SHRINK_THRESHOLD; + if (remaining_len == 0) { this.buf.clearRetainingCapacity(); this.total_bytes_written = 0; } else { - bun.copy(u8, this.buf.items[0..remaining_len], slice); + const slice = this.buf.items[SHRINK_THRESHOLD..this.total_bytes_written]; + std.mem.copyForwards(u8, this.buf.items[0..remaining_len], slice); this.buf.items.len = remaining_len; - this.total_bytes_written = 0; + this.total_bytes_written = remaining_len; } this.writers.truncate(this.__idx); this.__idx = 0; @@ -9331,7 +9208,7 @@ pub const Interpreter = struct { pub fn asyncDeinit(this: *@This()) void { print("IOWriter(0x{x}, fd={}) asyncDeinit", .{ @intFromPtr(this), this.fd }); - this.async_deinit.enqueue(); + this.async_deinit.schedule(); } pub fn __deinit(this: *This) void { @@ -10085,11 +9962,11 @@ pub fn SmolList(comptime T: type, comptime INLINED_MAX: comptime_int) type { .inlined => { if (starting_idx >= this.inlined.len) return; const slice_to_move = this.inlined.items[starting_idx..this.inlined.len]; - bun.copy(T, this.inlined.items[0..starting_idx], slice_to_move); + std.mem.copyForwards(T, this.inlined.items[0..starting_idx], slice_to_move); }, .heap => { const new_len = this.heap.len - starting_idx; - bun.copy(T, this.heap.ptr[0..new_len], this.heap.ptr[starting_idx..this.heap.len]); + this.heap.replaceRange(0, starting_idx, this.heap.ptr[starting_idx..this.heap.len]) catch bun.outOfMemory(); this.heap.len = @intCast(new_len); }, } diff --git a/src/shell/shell.zig b/src/shell/shell.zig index f448d9793a..35e2b89211 100644 --- a/src/shell/shell.zig +++ b/src/shell/shell.zig @@ -150,11 +150,6 @@ pub fn Result(comptime T: anytype) type { pub const success: @This() = @This(){ .result = std.mem.zeroes(T), }; - - pub fn asErr(this: @This()) ?ShellErr { - if (this == .err) return this.err; - return null; - } }; } diff --git a/test/js/bun/shell/bunshell.test.ts b/test/js/bun/shell/bunshell.test.ts index 9a9f91e501..e0c908a464 100644 --- a/test/js/bun/shell/bunshell.test.ts +++ b/test/js/bun/shell/bunshell.test.ts @@ -793,7 +793,8 @@ describe("deno_task", () => { test("stacktrace", async () => { // const folder = TestBuilder.tmpdir(); - const code = /* ts */ `import { $ } from 'bun' + const code = /* ts */ ` + import { $ } from 'bun' $.throws(true) @@ -816,79 +817,6 @@ describe("deno_task", () => { .stdout(s => expect(s).toInclude(`[eval]:${lineNr}`)) .run(); }); - - test("big_data", async () => { - const writerCode = /* ts */ ` - - const writer = Bun.stdout.writer(); - const buf = new Uint8Array(128 * 1024).fill('a'.charCodeAt(0)) - for (let i = 0; i < 10; i++) { - writer.write(buf); - } - await writer.flush() - `; - - const runnerCode = /* ts */ `await Bun.$\`BUN_DEBUG_QUIET_LOGS=1 ${BUN} -e ${$.escape(writerCode)} | cat\``; - const { stdout, stderr, exitCode } = await $`${BUN} -e ${runnerCode}`.env(bunEnv); - - expect(stderr.length).toEqual(0); - expect(exitCode).toEqual(0); - expect(stdout.length).toEqual(128 * 1024 * 10); - }); - - // https://github.com/oven-sh/bun/issues/9458 - test("input", async () => { - const inputCode = /* ts */ ` - const downArrow = '\\x1b[B'; - const enterKey = '\\x0D'; - await Bun.sleep(100) - const writer = Bun.stdout.writer(); - writer.write(downArrow) - await Bun.sleep(100) - writer.write(enterKey) - writer.flush() - `; - - const code = /* ts */ ` - const { select } = require('@inquirer/prompts'); - - async function run() { - const foobar = await select({ - message: 'Foo or Bar', - choices: [ - { name: 'Foo', value: 'foo' }, - { name: 'Bar', value: 'bar' }, - ], - }); - console.error('Choice:', foobar); - } - - run(); - `; - - const packagejson = ` - { - "name": "stuff", - "module": "index.ts", - "type": "module", - "devDependencies": { - "@types/bun": "latest" - }, - "peerDependencies": { - "typescript": "^5.0.0" - }, - "dependencies": { - "@inquirer/prompts": "v4.3.0" - } - } - `; - - await TestBuilder.command`echo ${packagejson} > package.json; BUN_DEBUG_QUIET_LOGS=1 ${BUN} i &> /dev/null; BUN_DEBUG_QUIET_LOGS=1 ${BUN} -e ${inputCode} | BUN_DEBUG_QUIET_LOGS=1 ${BUN} -e ${code}` - .ensureTempDir() - .stdout(() => {}) - .stderr("Choice: bar\n") - .run(); - }); }); function stringifyBuffer(buffer: Uint8Array): string { diff --git a/test/js/bun/shell/leak.test.ts b/test/js/bun/shell/leak.test.ts index fbfb4992a3..d1274fc2f9 100644 --- a/test/js/bun/shell/leak.test.ts +++ b/test/js/bun/shell/leak.test.ts @@ -64,7 +64,7 @@ describe("fd leak", () => { Bun.gc(true); const fd = openSync(devNull, "r"); closeSync(fd); - expect(fd - baseline).toBeLessThanOrEqual(threshold); + expect(Math.abs(fd - baseline)).toBeLessThanOrEqual(threshold); }, 100_000); } @@ -83,6 +83,7 @@ describe("fd leak", () => { writeFileSync(tempfile, testcode); const impl = /* ts */ ` + test("${name}", async () => { const threshold = ${threshold} let prev: number | undefined = undefined; let prevprev: number | undefined = undefined; @@ -98,9 +99,11 @@ describe("fd leak", () => { prev = val; prevprev = val; } else { + expect(Math.abs(prev - val)).toBeLessThan(threshold) if (!(Math.abs(prev - val) < threshold)) process.exit(1); } } + }, 1_000_000) `; appendFileSync(tempfile, impl);