diff --git a/packages/bun-types/bun.d.ts b/packages/bun-types/bun.d.ts index bed018c9a5043..5b5a316fa4e88 100644 --- a/packages/bun-types/bun.d.ts +++ b/packages/bun-types/bun.d.ts @@ -4615,6 +4615,32 @@ declare module "bun" { * @default cmds[0] */ argv0?: string; + + /** + * An {@link AbortSignal} that can be used to abort the subprocess. + * + * This is useful for aborting a subprocess when some other part of the + * program is aborted, such as a `fetch` response. + * + * Internally, this works by calling `subprocess.kill(1)`. + * + * @example + * ```ts + * const controller = new AbortController(); + * const { signal } = controller; + * const start = performance.now(); + * const subprocess = Bun.spawn({ + * cmd: ["sleep", "100"], + * signal, + * }); + * await Bun.sleep(1); + * controller.abort(); + * await subprocess.exited; + * const end = performance.now(); + * console.log(end - start); // 1ms instead of 101ms + * ``` + */ + signal?: AbortSignal; } type OptionsToSubprocess = diff --git a/src/bun.js/api/bun/subprocess.zig b/src/bun.js/api/bun/subprocess.zig index f51669b5948db..e5b211830c9e7 100644 --- a/src/bun.js/api/bun/subprocess.zig +++ b/src/bun.js/api/bun/subprocess.zig @@ -203,6 +203,7 @@ pub const Subprocess = struct { weak_file_sink_stdin_ptr: ?*JSC.WebCore.FileSink = null, ref_count: u32 = 1, + abort_signal: ?*JSC.AbortSignal = null, usingnamespace bun.NewRefCounted(@This(), Subprocess.deinit); @@ -226,6 +227,12 @@ pub const Subprocess = struct { poll_ref: Async.KeepAlive = .{}, }; + pub fn onAbortSignal(subprocess_ctx: ?*anyopaque, _: JSC.JSValue) callconv(.C) void { + var this: *Subprocess = @ptrCast(@alignCast(subprocess_ctx.?)); + this.clearAbortSignal(); + _ = this.tryKill(SignalCode.default); + } + pub fn resourceUsage( this: *Subprocess, globalObject: *JSGlobalObject, @@ -1437,6 +1444,7 @@ pub const Subprocess = struct { this_jsvalue.ensureStillAlive(); this.pid_rusage = rusage.*; const is_sync = this.flags.is_sync; + this.clearAbortSignal(); defer this.deref(); defer this.disconnectIPC(true); @@ -1587,6 +1595,15 @@ pub const Subprocess = struct { this.destroy(); } + fn clearAbortSignal(this: *Subprocess) void { + if (this.abort_signal) |signal| { + this.abort_signal = null; + signal.pendingActivityUnref(); + signal.cleanNativeBindings(this); + signal.unref(); + } + } + pub fn finalize(this: *Subprocess) callconv(.C) void { log("finalize", .{}); // Ensure any code which references the "this" value doesn't attempt to @@ -1594,6 +1611,8 @@ pub const Subprocess = struct { // access GC'd values during the finalizer this.this_jsvalue = .zero; + this.clearAbortSignal(); + bun.assert(!this.hasPendingActivity() or JSC.VirtualMachine.get().isShuttingDown()); this.finalizeStreams(); @@ -1702,6 +1721,13 @@ pub const Subprocess = struct { var windows_hide: bool = false; var windows_verbatim_arguments: bool = false; + var abort_signal: ?*JSC.WebCore.AbortSignal = null; + defer { + // Ensure we clean it up on error. + if (abort_signal) |signal| { + signal.unref(); + } + } { if (args.isEmptyOrUndefinedOrNull()) { @@ -1846,6 +1872,14 @@ pub const Subprocess = struct { } } + if (args.getOwnTruthy(globalThis, "signal")) |signal_val| { + if (signal_val.as(JSC.WebCore.AbortSignal)) |signal| { + abort_signal = signal.ref(); + } else { + return globalThis.throwInvalidArgumentTypeValue("signal", "AbortSignal", signal_val); + } + } + if (args.getOwnTruthy(globalThis, "onDisconnect")) |onDisconnect_| { if (!onDisconnect_.isCell() or !onDisconnect_.isCallable(globalThis.vm())) { globalThis.throwInvalidArguments("onDisconnect must be a function or undefined", .{}); @@ -2280,12 +2314,29 @@ pub const Subprocess = struct { should_close_memfd = false; if (comptime !is_sync) { + // Once everything is set up, we can add the abort listener + // Adding the abort listener may call the onAbortSignal callback immediately if it was already aborted + // Therefore, we must do this at the very end. + if (abort_signal) |signal| { + signal.pendingActivityRef(); + subprocess.abort_signal = signal.addListener(subprocess, onAbortSignal); + abort_signal = null; + } return out; } if (comptime is_sync) { switch (subprocess.process.watchOrReap()) { - .result => {}, + .result => { + // Once everything is set up, we can add the abort listener + // Adding the abort listener may call the onAbortSignal callback immediately if it was already aborted + // Therefore, we must do this at the very end. + if (abort_signal) |signal| { + signal.pendingActivityRef(); + subprocess.abort_signal = signal.addListener(subprocess, onAbortSignal); + abort_signal = null; + } + }, .err => { subprocess.process.wait(true); }, diff --git a/test/js/bun/spawn/spawn-signal.test.ts b/test/js/bun/spawn/spawn-signal.test.ts new file mode 100644 index 0000000000000..535bd23964bac --- /dev/null +++ b/test/js/bun/spawn/spawn-signal.test.ts @@ -0,0 +1,93 @@ +import { test, expect } from "bun:test"; +import { spawn, sleep } from "bun"; +import { bunEnv, bunExe } from "harness"; + +test("spawn AbortSignal works after spawning", async () => { + const controller = new AbortController(); + const { signal } = controller; + const start = performance.now(); + const subprocess = Bun.spawn({ + cmd: [bunExe(), "--eval", "await Bun.sleep(100000)"], + env: bunEnv, + stdout: "inherit", + stderr: "inherit", + stdin: "inherit", + signal, + }); + await Bun.sleep(1); + controller.abort(); + expect(await subprocess.exited).not.toBe(0); + const end = performance.now(); + expect(end - start).toBeLessThan(100); +}); + +test("spawn AbortSignal works if already aborted", async () => { + const controller = new AbortController(); + const { signal } = controller; + const start = performance.now(); + const subprocess = Bun.spawn({ + cmd: [bunExe(), "--eval", "await Bun.sleep(100000)"], + env: bunEnv, + stdout: "inherit", + stderr: "inherit", + stdin: "inherit", + signal, + }); + await Bun.sleep(1); + controller.abort(); + expect(await subprocess.exited).not.toBe(0); + const end = performance.now(); + expect(end - start).toBeLessThan(100); +}); + +test("spawn AbortSignal args validation", async () => { + expect(() => + Bun.spawn({ + cmd: [bunExe(), "--eval", "await Bun.sleep(100000)"], + env: bunEnv, + stdout: "inherit", + stderr: "inherit", + stdin: "inherit", + signal: 123, + }), + ).toThrow(); +}); + +test("spawnSync AbortSignal works as timeout", async () => { + const start = performance.now(); + const subprocess = Bun.spawnSync({ + cmd: [bunExe(), "--eval", "await Bun.sleep(100000)"], + env: bunEnv, + stdout: "inherit", + stderr: "inherit", + stdin: "inherit", + signal: AbortSignal.timeout(10), + }); + + expect(subprocess.success).toBeFalse(); + const end = performance.now(); + expect(end - start).toBeLessThan(100); +}); + +// TODO: this test should fail. +// It passes because we are ticking the event loop incorrectly in spawnSync. +// it should be ticking a different event loop. +test("spawnSync AbortSignal...executes javascript?", async () => { + const start = performance.now(); + var signal = AbortSignal.timeout(10); + signal.addEventListener("abort", () => { + console.log("abort", performance.now()); + }); + const subprocess = Bun.spawnSync({ + cmd: [bunExe(), "--eval", "await Bun.sleep(100000)"], + env: bunEnv, + stdout: "inherit", + stderr: "inherit", + stdin: "inherit", + signal, + }); + console.log("after", performance.now()); + expect(subprocess.success).toBeFalse(); + const end = performance.now(); + expect(end - start).toBeLessThan(100); +});