From 28006d0ad469a06c0e0bd7a2cfd90ef06d7e8991 Mon Sep 17 00:00:00 2001 From: 190n Date: Sat, 14 Jun 2025 07:59:29 -0700 Subject: [PATCH] WIP: Fix ref counting bugs in spawn API observed under rr (#20191) Co-authored-by: Jarred Sumner Co-authored-by: Jarred-Sumner <709451+Jarred-Sumner@users.noreply.github.com> --- src/bun.js/ProcessAutoKiller.zig | 21 +++++++++++++++++---- src/bun.js/api/bun/process.zig | 7 ++++++- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/bun.js/ProcessAutoKiller.zig b/src/bun.js/ProcessAutoKiller.zig index a4f3259072..96930e4ebe 100644 --- a/src/bun.js/ProcessAutoKiller.zig +++ b/src/bun.js/ProcessAutoKiller.zig @@ -41,6 +41,7 @@ pub fn kill(this: *ProcessAutoKiller) Result { fn killProcesses(this: *ProcessAutoKiller) u32 { var count: u32 = 0; while (this.processes.pop()) |process| { + defer process.key.deref(); if (!process.key.hasExited()) { log("process.kill {d}", .{process.key.pid}); count += @as(u32, @intFromBool(process.key.kill(@intFromEnum(bun.SignalCode.default)) == .result)); @@ -50,6 +51,10 @@ fn killProcesses(this: *ProcessAutoKiller) u32 { } pub fn clear(this: *ProcessAutoKiller) void { + for (this.processes.keys()) |process| { + process.deref(); + } + if (this.processes.capacity() > 256) { this.processes.clearAndFree(bun.default_allocator); } @@ -58,15 +63,23 @@ pub fn clear(this: *ProcessAutoKiller) void { } pub fn onSubprocessSpawn(this: *ProcessAutoKiller, process: *bun.spawn.Process) void { - if (this.enabled) - this.processes.put(bun.default_allocator, process, {}) catch {}; + if (this.enabled) { + this.processes.put(bun.default_allocator, process, {}) catch return; + process.ref(); + } } pub fn onSubprocessExit(this: *ProcessAutoKiller, process: *bun.spawn.Process) void { - if (this.ever_enabled) - _ = this.processes.swapRemove(process); + if (this.ever_enabled) { + if (this.processes.swapRemove(process)) { + process.deref(); + } + } } pub fn deinit(this: *ProcessAutoKiller) void { + for (this.processes.keys()) |process| { + process.deref(); + } this.processes.deinit(bun.default_allocator); } diff --git a/src/bun.js/api/bun/process.zig b/src/bun.js/api/bun/process.zig index 56477eeb7b..1d079e128b 100644 --- a/src/bun.js/api/bun/process.zig +++ b/src/bun.js/api/bun/process.zig @@ -374,11 +374,16 @@ pub const Process = struct { } if (this.poller == .fd) { - return this.poller.fd.register( + const maybe = this.poller.fd.register( this.event_loop.loop(), .process, true, ); + switch (maybe) { + .err => {}, + .result => this.ref(), + } + return maybe; } else { @panic("Internal Bun error: poll_ref in Subprocess is null unexpectedly. Please file a bug report."); }