Merge pull request #152 from redpanda-data/mboquard/core-8181/waitpid…

…-workaround reactor: Always retry waitpid
redpanda-data · Nov 11, 2024 · 2380681 · 2380681
2 parents f212c77 + 09b0746
commit 2380681
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 32 deletions.
diff --git a/include/seastar/core/reactor.hh b/include/seastar/core/reactor.hh
@@ -642,6 +642,7 @@ 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();

diff --git a/src/core/reactor.cc b/src/core/reactor.cc
@@ -2296,6 +2296,32 @@ 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] {
         return wrap_syscall<int>(syscall(__NR_pidfd_open, pid, O_NONBLOCK));
@@ -2304,39 +2330,11 @@ future<int> reactor::waitpid(pid_t 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().
-            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);
-                        }
-                    });
-                });
-            });
+            return do_waitpid(pid);
         } else {
-            return do_with(pollable_fd(file_desc::from_fd(pidfd.result)), int{}, [pid, this](auto& pidfd, int& wstatus) {
-                return pidfd.readable().then([pid, &wstatus, this] {
-                    return _thread_pool->submit<syscall_result<pid_t>>([pid, &wstatus] {
-                        return wrap_syscall<pid_t>(::waitpid(pid, &wstatus, WNOHANG));
-                    });
-                }).then([&wstatus] (syscall_result<pid_t> ret) {
-                    ret.throw_if_error();
-                    assert(ret.result > 0);
-                    return make_ready_future<int>(wstatus);
+            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);
                 });
             });
         }