Hi! I’ve recently been implementing process waiting in my interpreter, and I think the current Threaded implementation of wait leaks tasks when canceling. Here’s a reproducer:
const std = @import("std");
const Io = std.Io;
fn task(io: std.Io) void {
// 1. Spawn process
var child = std.process.spawn(io, .{
.argv = &.{ "sleep", "60" },
}) catch unreachable;
defer child.kill(io); // 3. This becomes a no-op, since `child.id` was cleared.
// 2. This `wait` call will be canceled by `main`. This will clear `child.id`,
// but will _not_ kill `child`.
_ = child.wait(io) catch |err| switch (err) {
error.Canceled => std.debug.print("Canceled\n", .{}),
else => {},
};
}
pub fn main(init: std.process.Init) !void {
const io = init.io;
var spawned = try io.concurrent(task, .{io});
spawned.cancel(io);
spawned.await(io);
// Give the task a chance to clean up.
try io.sleep(.fromMilliseconds(500), .awake);
// Print live children.
var dir = try Io.Dir.cwd().openDir(io, "/proc/self/task", .{ .iterate = true });
defer dir.close(io);
var it = dir.iterate();
while (try it.next(io)) |entry| {
var buf: [4096]u8 = undefined;
var path: [64]u8 = undefined;
const p = try std.fmt.bufPrint(&path, "{s}/children", .{entry.name});
std.debug.print("thread {s} has processes: {s}\n", .{ entry.name, try dir.readFile(io, p, &buf) });
}
}
When I run this reproducer, I get:
Canceled
thread 145321 has processes:
thread 145337 has processes: 145338
So there’s still a process hanging around, even though I used child.kill() for cleanup.
The issue is here (in Threaded.zig):
fn childCleanupPosix(child: *process.Child) void {
if (child.stdin) |stdin| {
closeFd(stdin.handle);
child.stdin = null;
}
if (child.stdout) |stdout| {
closeFd(stdout.handle);
child.stdout = null;
}
if (child.stderr) |stderr| {
closeFd(stderr.handle);
child.stderr = null;
}
child.id = null;
}
fn childWaitPosix(child: *process.Child) process.Child.WaitError!process.Child.Term {
// Closes the child's fds, clears `id`, but does _not_ kill the task.
defer childCleanupPosix(child);
const pid = child.id.?;
var ru: posix.rusage = undefined;
const ru_ptr = if (child.request_resource_usage_statistics) &ru else null;
if (have_wait4) {
var status: if (builtin.link_libc) c_int else u32 = undefined;
const syscall: Syscall = try .start();
while (true) switch (posix.errno(posix.system.wait4(pid, &status, 0, ru_ptr))) {
.SUCCESS => {
syscall.finish();
if (ru_ptr) |p| child.resource_usage_statistics.rusage = p.*;
return statusToTerm(@bitCast(status));
},
.INTR => {
// returns error.Canceled when canceled, but also doesn't kill the process.
try syscall.checkCancel();
continue;
},
.CHILD => |err| return syscall.errnoBug(err),
else => |err| return syscall.unexpectedErrno(err),
};
}
// ...
}
The reason I have this as a forum post instead of a bug report is that these are the semantics that are already documented for process.Child.id: “After wait or kill is called, this becomes null.”
So, should wait kill the process if wait gets canceled? Or should wait not clear id in the case of cancelation? Or perhaps something else?