diff options
author | 2023-09-16 22:44:13 -0700 | |
---|---|---|
committer | 2023-09-16 22:44:13 -0700 | |
commit | 4e0c589562abc0f2e91b428fa0fea8850a7afacf (patch) | |
tree | 50a4c285c8b372fac07578f4227e9231493436b0 | |
parent | a098c6e5f6d63a9bfd1430798fce4896e26f1a9f (diff) | |
download | bun-4e0c589562abc0f2e91b428fa0fea8850a7afacf.tar.gz bun-4e0c589562abc0f2e91b428fa0fea8850a7afacf.tar.zst bun-4e0c589562abc0f2e91b428fa0fea8850a7afacf.zip |
fix(child_process) unref next tick so exit/close event can be fired before application exits (#5481)
* unref next tick so exit callback can be called
* fmt + test
* oops
* add ref_count
* update pending
* comment and fix
-rw-r--r-- | src/bun.js/api/bun/subprocess.zig | 32 | ||||
-rw-r--r-- | test/js/node/child_process/child_process.test.ts | 22 | ||||
-rw-r--r-- | test/js/node/child_process/fixtures/child-process-exit-event.js | 13 |
3 files changed, 63 insertions, 4 deletions
diff --git a/src/bun.js/api/bun/subprocess.zig b/src/bun.js/api/bun/subprocess.zig index 5695c15ad..b6e27d287 100644 --- a/src/bun.js/api/bun/subprocess.zig +++ b/src/bun.js/api/bun/subprocess.zig @@ -68,6 +68,7 @@ pub const Subprocess = struct { ipc_callback: JSC.Strong = .{}, ipc_buffer: bun.ByteList, + has_pending_unref: bool = false, pub const SignalCode = bun.SignalCode; pub const IPCMode = enum { @@ -82,7 +83,7 @@ pub const Subprocess = struct { pub fn updateHasPendingActivityFlag(this: *Subprocess) void { @fence(.SeqCst); - this.has_pending_activity.store(this.waitpid_err == null and this.exit_code == null and this.ipc == .none, .SeqCst); + this.has_pending_activity.store(this.waitpid_err == null and this.exit_code == null and this.ipc == .none and this.has_pending_unref, .SeqCst); } pub fn hasPendingActivity(this: *Subprocess) callconv(.C) bool { @@ -92,7 +93,7 @@ pub const Subprocess = struct { pub fn updateHasPendingActivity(this: *Subprocess) void { @fence(.Release); - this.has_pending_activity.store(this.waitpid_err == null and this.exit_code == null and this.ipc == .none, .Release); + this.has_pending_activity.store(this.waitpid_err == null and this.exit_code == null and this.ipc == .none and this.has_pending_unref, .Release); } pub fn ref(this: *Subprocess) void { @@ -111,8 +112,10 @@ pub const Subprocess = struct { } } + /// This disables the keeping process alive flag on the poll and also in the stdin, stdout, and stderr pub fn unref(this: *Subprocess) void { var vm = this.globalThis.bunVM(); + if (this.poll_ref) |poll| poll.disableKeepingProcessAlive(vm); if (!this.hasCalledGetter(.stdin)) { this.stdin.unref(); @@ -1805,8 +1808,29 @@ pub const Subprocess = struct { } } - if (this.hasExited()) - this.unref(); + if (this.hasExited()) { + const Holder = struct { + process: *Subprocess, + task: JSC.AnyTask, + + pub fn unref(self: *@This()) void { + // this calls disableKeepingProcessAlive on pool_ref and stdin, stdout, stderr + self.process.unref(); + self.process.has_pending_unref = false; + self.process.updateHasPendingActivity(); + bun.default_allocator.destroy(self); + } + }; + + var holder = bun.default_allocator.create(Holder) catch @panic("OOM"); + this.has_pending_unref = true; + holder.* = .{ + .process = this, + .task = JSC.AnyTask.New(Holder, Holder.unref).init(holder), + }; + + this.globalThis.bunVM().enqueueTask(JSC.Task.init(&holder.task)); + } } const os = std.os; diff --git a/test/js/node/child_process/child_process.test.ts b/test/js/node/child_process/child_process.test.ts index baf422bc9..8c24ef55e 100644 --- a/test/js/node/child_process/child_process.test.ts +++ b/test/js/node/child_process/child_process.test.ts @@ -2,6 +2,8 @@ import { describe, it, expect } from "bun:test"; import { ChildProcess, spawn, execFile, exec, fork, spawnSync, execFileSync, execSync } from "node:child_process"; import { tmpdir } from "node:os"; import { promisify } from "node:util"; +import { bunExe, bunEnv } from "harness"; +import path from "path"; const debug = process.env.DEBUG ? console.log : () => {}; @@ -308,3 +310,23 @@ describe("Bun.spawn()", () => { // expect(child.pid).toBe(undefined); // }); }); + +it("should call close and exit before process exits", async () => { + const proc = Bun.spawn({ + cmd: [bunExe(), path.join("fixtures", "child-process-exit-event.js")], + cwd: import.meta.dir, + env: bunEnv, + stdout: "pipe", + }); + await proc.exited; + expect(proc.exitCode).toBe(0); + let data = ""; + const reader = proc.stdout.getReader(); + while (true) { + const { done, value } = await reader.read(); + if (done) break; + data += new TextDecoder().decode(value); + } + expect(data).toContain("closeHandler called"); + expect(data).toContain("exithHandler called"); +}); diff --git a/test/js/node/child_process/fixtures/child-process-exit-event.js b/test/js/node/child_process/fixtures/child-process-exit-event.js new file mode 100644 index 000000000..4400ace1b --- /dev/null +++ b/test/js/node/child_process/fixtures/child-process-exit-event.js @@ -0,0 +1,13 @@ +const { spawn } = require("node:child_process"); + +function exitHandler() { + console.log("exithHandler called"); +} +function closeHandler() { + console.log("closeHandler called"); +} + +const p = spawn("bun", ["--version"]); + +p.on("exit", exitHandler); +p.on("close", closeHandler); |