From aad6f885957a31774ee94e5cdf5925d53152e954 Mon Sep 17 00:00:00 2001
From: Drew DeVault <drew@ddevault.org>
Date: Thu, 27 Feb 2025 17:02:18 +0100
Subject: [PATCH] Simplify ev::wait API

By testing for expected error cases upfront.

Signed-off-by: Drew DeVault <drew@ddevault.org>

---
 cmd/child/main.ha    | 13 ++++---------
 ev/+linux/process.ha | 76 +++++++++++++++++++++++++++++++++++++++--------------

diff --git a/cmd/child/main.ha b/cmd/child/main.ha
index ec9582a20ab449b3835d4e6b69ca2b839c8e5e28..9808ba82871f7a43fb6a0c6af88ddf4579db6c25 100644
--- a/cmd/child/main.ha
+++ b/cmd/child/main.ha
@@ -11,7 +11,7 @@ 	defer ev::finish(&loop);
 
 	const cmd = exec::cmd(os::args[1], os::args[2..]...)!;
 	const child = ev::exec(&loop, &cmd)!;
-	ev::wait(child, &child_exited);
+	ev::wait(child, &child_exited)!;
 
 	const timer = ev::newtimer(&loop, &expired, time::clock::MONOTONIC)!;
 	ev::timer_configure(timer, 5 * time::SECOND, 0);
@@ -26,14 +26,9 @@ 	const child = ev::getuser(timer): *ev::file;
 	ev::kill(child)!;
 };
 
-fn child_exited(child: *ev::file, r: (exec::status | errors::error)) void = {
-	match (r) {
-	case let st: exec::status =>
-		const exit = exec::exit(&st);
-		fmt::printfln("child exited: {}", exec::exitstr(exit))!;
-	case let err: errors::error =>
-		fmt::printfln("ev::wait: {}", errors::strerror(err))!;
-	};
+fn child_exited(child: *ev::file, r: exec::status) void = {
+	const exit = exec::exit(&r);
+	fmt::printfln("child exited: {}", exec::exitstr(exit))!;
 
 	const loop = ev::getloop(child);
 	ev::stop(loop);
diff --git a/ev/+linux/process.ha b/ev/+linux/process.ha
index 2a950d2500ae40c2987cdc8cc8cc638e683e1a95..2c325b9a8931372a4e54f5c2b272bea12033e800 100644
--- a/ev/+linux/process.ha
+++ b/ev/+linux/process.ha
@@ -14,7 +14,7 @@ 	let pidfd: int = 0;
 	let args = rt::clone_args {
 		flags = rt::CLONE_PIDFD,
 		pidfd = &pidfd: uintptr: u64,
-		exit_signal = 0,
+		exit_signal = sig::CHLD: u64,
 		...
 	};
 
@@ -33,39 +33,50 @@ 	return register(loop, pidfd);
 };
 
 // A callback for a [[wait]] operation.
-export type waitcb = fn(file: *file, result: (exec::status | errors::error)) void;
+export type waitcb = fn(file: *file, result: exec::status) void;
 
 // Waits for a process to exit. After the callback is run, the [[file]] is
 // closed and unregistered from the event loop and may not be used again.
-export fn wait(proc: *file, cb: *waitcb) req = {
+export fn wait(proc: *file, cb: *waitcb) (req | errors::error) = {
 	assert(proc.op & op::WAIT == 0);
+
+	// Attempt to waitid now to hit error cases early
+	match (do_waitid(proc, cb, rt::WNOHANG)) {
+	case let err: rt::errno =>
+		// We expect EAGAIN, anything else indicates that something went
+		// wrong (e.g. EBADF if using a non-pidfd with ev::wait)
+		if (err != rt::EAGAIN) {
+			return errors::errno(err);
+		};
+	case void =>
+		// Child has already exited. do_waitid ran the callback, so we
+		// can return with no further fanfare
+		return req { ... };
+	};
+
 	proc.op |= op::WAIT;
 	proc.cb = cb;
 	file_epoll_ctl(proc);
 	return mkreq(&wait_cancel, proc);
 };
 
-fn wait_cancel(req: *req) void = {
-	const proc = req.user: *file;
-	assert(proc.op & op::WAIT != 0);
-	proc.op &= ~op::WAIT;
-	file_epoll_ctl(proc);
-};
-
-fn wait_ready(proc: *file, ev: *rt::epoll_event) void = {
-	assert(proc.op & op::WAIT != 0);
-	assert(proc.cb != null);
-	const cb = proc.cb: *waitcb;
-	proc.op &= ~op::WAIT;
-	file_epoll_ctl(proc);
+// Calls [[rt::waitid]] with the given options. If successful, calls the
+// callback and cleans up the file (and returns void). If unsuccessful, returns
+// the error.
+fn do_waitid(proc: *file, cb: *waitcb, options: int = 0) (void | rt::errno) = {
+	def WAIT_OPTIONS = rt::WEXITED | rt::WSTOPPED | rt::WCONTINUED;
 
 	let si = rt::siginfo { ... };
 	let ru = rt::rusage { ... };
 	match (rt::waitid(rt::idtype::P_PIDFD, proc.fd: rt::id_t, &si,
-		rt::WEXITED | rt::WSTOPPED | rt::WCONTINUED, &ru)) {
+		WAIT_OPTIONS | options, &ru)) {
 	case let err: rt::errno =>
-		cb(proc, errors::errno(err));
-	case void => void;
+		return err;
+	case void =>
+		if (si.si_pid == 0) {
+			assert(options & rt::WNOHANG != 0);
+			return rt::EAGAIN;
+		};
 	};
 
 	// Convert the siginfo data into a wait(2)-style exit status
@@ -82,8 +93,33 @@ 		...
 	};
 	rusage(&st, &ru);
 	cb(proc, st);
-
 	close(proc);
+};
+
+fn wait_cancel(req: *req) void = {
+	const proc = req.user: *file;
+	assert(proc.op & op::WAIT != 0);
+	proc.op &= ~op::WAIT;
+	file_epoll_ctl(proc);
+};
+
+fn wait_ready(proc: *file, ev: *rt::epoll_event) void = {
+	assert(proc.op & op::WAIT != 0);
+	assert(proc.cb != null);
+	const cb = proc.cb: *waitcb;
+	proc.op &= ~op::WAIT;
+	file_epoll_ctl(proc);
+
+	// waitid should not fail at this point
+	for (true) {
+		match (do_waitid(proc, cb)) {
+		case void =>
+			break;
+		case let err: rt::errno =>
+			// Interrupted by signal handler, go again
+			assert(err == rt::EINTR);
+		};
+	};
 };
 
 // Copied from os::exec

-- 
2.48.1