reactor: coroutinize waitpid

Commit fb6c969 simplified
waitpid by always retrying, even if pidfd_open failed.

Coroutinizing this path (and folding do_waitpid back into
its only caller: waitpid) simplifies the code even further
and consolidate the diffrent do_with allocations into a single
coroutine frame.

Signed-off-by: Benny Halevy <[email protected]>

include/seastar/core/reactor.hh

@@ -599,7 +599,6 @@ private:
     void add_timer(timer<manual_clock>*) noexcept;
     bool queue_timer(timer<manual_clock>*) noexcept;
     void del_timer(timer<manual_clock>*) noexcept;
-    future<int> do_waitpid(pid_t pid);
 
     future<> run_exit_tasks();
     void stop();

src/core/reactor.cc

@@ -2075,49 +2075,34 @@ static auto next_waitpid_timeout(std::chrono::milliseconds this_timeout) {
 
 #endif
 
-future<int> reactor::do_waitpid(pid_t pid) {
-    return do_with(int{}, std::chrono::milliseconds(0), [pid, this](int& wstatus,
-                                                                    std::chrono::milliseconds& wait_timeout) {
-        return repeat_until_value([this,
-                                   pid,
-                                   &wstatus,
-                                   &wait_timeout] {
-            return _thread_pool->submit<syscall_result<pid_t>>([pid, &wstatus] {
-                return wrap_syscall<pid_t>(::waitpid(pid, &wstatus, WNOHANG));
-            }).then([&wstatus, &wait_timeout](syscall_result<pid_t> ret) mutable {
-                if (ret.result == 0) {
-                    wait_timeout = next_waitpid_timeout(wait_timeout);
-                    return ::seastar::sleep(wait_timeout).then([] {
-                        return make_ready_future<std::optional<int>>();
-                    });
-                } else if (ret.result > 0) {
-                    return make_ready_future<std::optional<int>>(wstatus);
-                } else {
-                    ret.throw_if_error();
-                    return make_ready_future<std::optional<int>>(-1);
-                }
-            });
-        });
-    });
-}
-
 future<int> reactor::waitpid(pid_t pid) {
-    return _thread_pool->submit<syscall_result<int>>([pid] {
+    syscall_result<int> pidfd = co_await _thread_pool->submit<syscall_result<int>>([pid] {
         return wrap_syscall<int>(syscall(__NR_pidfd_open, pid, O_NONBLOCK));
-    }).then([pid, this] (syscall_result<int> pidfd) {
-        if (pidfd.result == -1) {
-            // pidfd_open() was introduced in linux 5.3, so the pidfd.error could be ENOSYS on
-            // older kernels. But it could be other error like EMFILE or ENFILE. anyway, we
-            // should always waitpid().
-            return do_waitpid(pid);
-        } else {
-            return do_with(pollable_fd(file_desc::from_fd(pidfd.result)), [pid, this](auto& pidfd) {
-                return pidfd.readable().then([pid, this] {
-                    return do_waitpid(pid);
-                });
-            });
-        }
     });
+    // pidfd_open() was introduced in linux 5.3, so the pidfd.error could be ENOSYS on
+    // older kernels. But it could be other error like EMFILE or ENFILE. anyway, we
+    // should always waitpid().
+    std::optional<pollable_fd> pfd;
+    if (pidfd.result != -1) {
+        pfd.emplace(file_desc::from_fd(pidfd.result));
+        co_await pfd->readable();
+    }
+
+    std::chrono::milliseconds wait_timeout(0);
+    for (;;) {
+        int wstatus;
+        auto ret = co_await _thread_pool->submit<syscall_result<pid_t>>([pid, &wstatus] {
+            return wrap_syscall<pid_t>(::waitpid(pid, &wstatus, WNOHANG));
+        });
+        if (ret.result > 0) {
+            co_return wstatus;
+        } else if (ret.result < 0) {
+            ret.throw_if_error();
+            co_return -1;
+        }
+        wait_timeout = next_waitpid_timeout(wait_timeout);
+        co_await sleep(wait_timeout);
+    }
 }
 
 void reactor::kill(pid_t pid, int sig) {