From aa9e12dac2d11fc71f055bd5db61f18453fb5345 Mon Sep 17 00:00:00 2001 From: Will Pragnell Date: Wed, 30 Aug 2023 21:34:34 -0700 Subject: [PATCH 1/2] termio/exec: don't leak zombie subprocesses --- src/Command.zig | 9 ++++++++- src/termio/Exec.zig | 5 +++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/Command.zig b/src/Command.zig index 7bf64566e..12dea7f3a 100644 --- a/src/Command.zig +++ b/src/Command.zig @@ -190,7 +190,14 @@ pub fn wait(self: Command, block: bool) !Exit { // We specify NOHANG because its not our fault if the process we launch // for the tty doesn't properly waitpid its children. We don't want // to hang the terminal over it. - const res = std.os.waitpid(self.pid.?, if (block) 0 else std.c.W.NOHANG); + // When NOHANG is specified, waitpid will return a pid of 0 if the process + // doesn't have a status to report. When that happens, it is as though the + // wait call has not been performed, so we need to keep trying until we get + // a non-zero pid back, otherwise we end up with zombie processes. + var res = std.os.WaitPidResult{ .pid = 0, .status = 0 }; + while (res.pid == 0) { + res = std.os.waitpid(self.pid.?, if (block) 0 else std.c.W.NOHANG); + } return Exit.init(res.status); } diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig index 95b5ab962..4df512e7e 100644 --- a/src/termio/Exec.zig +++ b/src/termio/Exec.zig @@ -831,8 +831,9 @@ const Subprocess = struct { } /// Stop the subprocess. This is safe to call anytime. This will wait - /// for the subprocess to end so it will block. This does not close - /// the pty. + /// for the subprocess to register that it has been signalled, but not + /// for it to terminate, so it will not block. + /// This does not close the pty. pub fn stop(self: *Subprocess) void { // Kill our command if (self.command) |*cmd| { From 2e54ad2ccefc2cd1b8832aad40e8d93884392372 Mon Sep 17 00:00:00 2001 From: Will Pragnell Date: Wed, 30 Aug 2023 22:02:25 -0700 Subject: [PATCH 2/2] command: only spin on waitpid if it's non-blocking --- src/Command.zig | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/Command.zig b/src/Command.zig index 12dea7f3a..8fa8808a5 100644 --- a/src/Command.zig +++ b/src/Command.zig @@ -187,17 +187,20 @@ fn setupFd(src: File.Handle, target: i32) !void { /// Wait for the command to exit and return information about how it exited. pub fn wait(self: Command, block: bool) !Exit { - // We specify NOHANG because its not our fault if the process we launch - // for the tty doesn't properly waitpid its children. We don't want - // to hang the terminal over it. - // When NOHANG is specified, waitpid will return a pid of 0 if the process - // doesn't have a status to report. When that happens, it is as though the - // wait call has not been performed, so we need to keep trying until we get - // a non-zero pid back, otherwise we end up with zombie processes. - var res = std.os.WaitPidResult{ .pid = 0, .status = 0 }; - while (res.pid == 0) { - res = std.os.waitpid(self.pid.?, if (block) 0 else std.c.W.NOHANG); - } + const res = if (block) std.os.waitpid(self.pid.?, 0) else res: { + // We specify NOHANG because its not our fault if the process we launch + // for the tty doesn't properly waitpid its children. We don't want + // to hang the terminal over it. + // When NOHANG is specified, waitpid will return a pid of 0 if the process + // doesn't have a status to report. When that happens, it is as though the + // wait call has not been performed, so we need to keep trying until we get + // a non-zero pid back, otherwise we end up with zombie processes. + while (true) { + const res = std.os.waitpid(self.pid.?, std.c.W.NOHANG); + if (res.pid != 0) break :res res; + } + }; + return Exit.init(res.status); }