diff --git a/src/termio/Exec.zig b/src/termio/Exec.zig index 7fb4f39b455..e97214ac344 100644 --- a/src/termio/Exec.zig +++ b/src/termio/Exec.zig @@ -1163,16 +1163,13 @@ const Subprocess = struct { }, } } else if (comptime builtin.os.tag != .windows) { - // The process watcher may already have consumed the direct child's - // wait status. Its process group remains ours to terminate. - if (self.process_group_id) |pgid| { - killProcessGroupsWithTimeouts( - pgid, - foreground_process_group_id, - null, - timeouts, - ) catch |err| - log.err("error stopping process group: {}", .{err}); + // Once the watcher consumes the direct child's wait status, its + // cached numeric process-group id may be recycled. A foreground + // group freshly observed through this retained PTY is the only + // group identity that remains attributable to this subprocess. + if (foreground_process_group_id) |pgid| { + killProcessGroupWithTimeouts(pgid, null, timeouts) catch |err| + log.err("error stopping foreground process group: {}", .{err}); } } @@ -2546,6 +2543,87 @@ test "subprocess stop kills descendants after the group leader was reaped" { try testing.expectEqual(posix.E.SRCH, group_probe_err); } +test "subprocess stop ignores a cached process group after the child was reaped" { + if (comptime builtin.os.tag == .windows or builtin.os.tag == .ios) { + return error.SkipZigTest; + } + + const testing = std.testing; + const c = Subprocess.c; + var pty = try Pty.open(.{}); + defer pty.deinit(); + + const ready_pipe = try internal_os.pipe(); + defer { + _ = posix.system.close(ready_pipe[0]); + _ = posix.system.close(ready_pipe[1]); + } + + const unrelated_pid: posix.pid_t = unrelated: { + const rc = posix.system.fork(); + switch (posix.errno(rc)) { + .SUCCESS => break :unrelated @intCast(rc), + .AGAIN, .NOMEM => return error.SystemResources, + else => |err| return posix.unexpectedErrno(err), + } + }; + if (unrelated_pid == 0) { + _ = posix.system.close(ready_pipe[0]); + if (c.setsid() < 0) c._exit(1); + + var action: posix.Sigaction = .{ + .handler = .{ .handler = posix.SIG.IGN }, + .mask = posix.sigemptyset(), + .flags = 0, + }; + posix.sigaction(posix.SIG.HUP, &action, null); + if (posix.system.write(ready_pipe[1], "r", 1) != 1) c._exit(1); + while (true) _ = c.pause(); + } + + var unrelated_reaped = false; + defer if (!unrelated_reaped) { + _ = c.killpg(unrelated_pid, c.SIGKILL); + var status: c_int = 0; + _ = posix.system.waitpid(unrelated_pid, &status, 0); + }; + _ = posix.system.close(ready_pipe[1]); + var ready: [1]u8 = undefined; + try testing.expectEqual(@as(usize, 1), try posix.read(ready_pipe[0], &ready)); + + var arena = ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + var subprocess: Subprocess = .{ + .arena = arena, + .cwd = null, + .env = null, + .args = &.{}, + .grid_size = .{}, + .screen_size = .{ .width = 1, .height = 1 }, + .pty = pty, + .process = null, + .process_group_id = unrelated_pid, + .rt_pre_exec_info = undefined, + .rt_post_fork_info = undefined, + }; + try testing.expectEqual( + @as(?c.pid_t, null), + subprocess.foregroundProcessGroupId(), + ); + + subprocess.stopWithTimeouts(.{ + .sighup_grace = .fromMilliseconds(20), + .sigkill_grace = .fromMilliseconds(100), + }); + + var status: c_int = 0; + const wait_result: posix.pid_t = @intCast( + posix.system.waitpid(unrelated_pid, &status, std.c.W.NOHANG), + ); + unrelated_reaped = wait_result == unrelated_pid; + try testing.expectEqual(@as(posix.pid_t, 0), wait_result); +} + test "subprocess stop kills a distinct foreground process group" { if (comptime builtin.os.tag == .windows or builtin.os.tag == .ios) { return error.SkipZigTest; @@ -2604,7 +2682,7 @@ test "subprocess stop kills a distinct foreground process group" { if (posix.system.read(job_ready_pipe[0], &job_ready, 1) != 1) c._exit(1); if (c.tcsetpgrp(pty.slave, @intCast(job_pid)) < 0) c._exit(1); if (posix.system.write(ready_pipe[1], "r", 1) != 1) c._exit(1); - while (true) _ = c.pause(); + c._exit(0); } var leader_reaped = false; @@ -2623,6 +2701,13 @@ test "subprocess stop kills a distinct foreground process group" { try testing.expectEqual(@as(usize, 1), try posix.read(ready_pipe[0], &ready)); _ = posix.system.close(pty.slave); + var leader_status: c_int = 0; + try testing.expectEqual( + leader_pid, + @as(posix.pid_t, @intCast(posix.system.waitpid(leader_pid, &leader_status, 0))), + ); + leader_reaped = true; + foreground_pgid = @intCast( pty.getProcessInfo(.foreground_pid) orelse return error.ForegroundProcessGroupUnavailable, @@ -2644,34 +2729,12 @@ test "subprocess stop kills a distinct foreground process group" { .rt_pre_exec_info = undefined, .rt_post_fork_info = undefined, }; - var leader_status: c_int = 0; - var wait_result: posix.pid_t = 0; - const leader_reaper = try std.Thread.spawn(.{}, struct { - fn run( - pid: posix.pid_t, - status: *c_int, - result: *posix.pid_t, - ) void { - result.* = @intCast(posix.system.waitpid(pid, status, 0)); - } - }.run, .{ leader_pid, &leader_status, &wait_result }); - var leader_reaper_joined = false; - defer if (!leader_reaper_joined) { - _ = c.killpg(leader_pid, c.SIGKILL); - leader_reaper.join(); - leader_reaped = true; - }; subprocess.stopWithTimeouts(.{ .sighup_grace = .fromMilliseconds(20), .sigkill_grace = .fromSeconds(1), }); - leader_reaper.join(); - leader_reaper_joined = true; - leader_reaped = wait_result == leader_pid; - try testing.expectEqual(leader_pid, wait_result); - const foreground_probe = c.killpg(foreground_pgid.?, 0); const foreground_probe_err = posix.errno(foreground_probe); try testing.expectEqual(@as(c_int, -1), foreground_probe);