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
This commit is contained in:
Ciro Spaciari
2023-09-16 22:44:13 -07:00
committed by GitHub
parent a098c6e5f6
commit 4e0c589562
3 changed files with 63 additions and 4 deletions

View File

@@ -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;

View File

@@ -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");
});

View File

@@ -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);