diff --git a/Kernel/Process.h b/Kernel/Process.h index 71d33f372e..00ab1e7ee9 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -541,7 +542,7 @@ private: KResult do_killall(int signal); KResult do_killself(int signal); - KResultOr do_waitid(idtype_t idtype, int id, int options); + KResultOr do_waitid(Variant, NonnullRefPtr> waitee, int options); KResultOr> get_syscall_path_argument(Userspace user_path, size_t path_length) const; KResultOr> get_syscall_path_argument(const Syscall::StringArgument&) const; diff --git a/Kernel/Syscalls/waitid.cpp b/Kernel/Syscalls/waitid.cpp index f33dd1d7e7..395859818b 100644 --- a/Kernel/Syscalls/waitid.cpp +++ b/Kernel/Syscalls/waitid.cpp @@ -4,15 +4,16 @@ * SPDX-License-Identifier: BSD-2-Clause */ +#include #include #include namespace Kernel { -KResultOr Process::do_waitid(idtype_t idtype, int id, int options) +KResultOr Process::do_waitid(Variant, NonnullRefPtr> waitee, int options) { KResultOr result = KResult(KSuccess); - if (Thread::current()->block({}, options, idtype, id, result).was_interrupted()) + if (Thread::current()->block({}, options, move(waitee), result).was_interrupted()) return EINTR; VERIFY(!result.is_error() || (options & WNOHANG) || result.error() != KSuccess); return result; @@ -27,18 +28,34 @@ KResultOr Process::sys$waitid(Userspace, NonnullRefPtr> waitee = Empty {}; switch (params.idtype) { case P_ALL: - case P_PID: - case P_PGID: + waitee = Empty {}; break; + case P_PID: { + auto waitee_process = Process::from_pid(params.id); + if (!waitee_process || waitee_process->ppid() != Process::current().pid()) { + return ECHILD; + } + waitee = waitee_process.release_nonnull(); + break; + } + case P_PGID: { + auto waitee_group = ProcessGroup::from_pgid(params.id); + if (!waitee_group) { + return ECHILD; + } + waitee = waitee_group.release_nonnull(); + break; + } default: return EINVAL; } dbgln_if(PROCESS_DEBUG, "sys$waitid({}, {}, {}, {})", params.idtype, params.id, params.infop, params.options); - auto siginfo_or_error = do_waitid(static_cast(params.idtype), params.id, params.options); + auto siginfo_or_error = do_waitid(move(waitee), params.options); if (siginfo_or_error.is_error()) return siginfo_or_error.error(); diff --git a/Kernel/Thread.h b/Kernel/Thread.h index 4c90d1f35e..dd815418cd 100644 --- a/Kernel/Thread.h +++ b/Kernel/Thread.h @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -705,7 +706,7 @@ public: Disowned }; - WaitBlocker(int wait_options, idtype_t id_type, pid_t id, KResultOr& result); + WaitBlocker(int wait_options, Variant, NonnullRefPtr> waitee, KResultOr& result); virtual StringView state_string() const override { return "Waiting"sv; } virtual Type blocker_type() const override { return Type::Wait; } virtual void will_unblock_immediately_without_blocking(UnblockImmediatelyReason) override; @@ -720,13 +721,9 @@ public: void do_set_result(const siginfo_t&); const int m_wait_options; - const idtype_t m_id_type; - const pid_t m_waitee_id; KResultOr& m_result; - RefPtr m_waitee; - RefPtr m_waitee_group; + Variant, NonnullRefPtr> m_waitee; bool m_did_unblock { false }; - bool m_error { false }; bool m_got_sigchild { false }; }; diff --git a/Kernel/ThreadBlockers.cpp b/Kernel/ThreadBlockers.cpp index 4d0c26d320..d4c8e971ea 100644 --- a/Kernel/ThreadBlockers.cpp +++ b/Kernel/ThreadBlockers.cpp @@ -601,55 +601,23 @@ void Thread::WaitBlockerSet::finalize() } } -Thread::WaitBlocker::WaitBlocker(int wait_options, idtype_t id_type, pid_t id, KResultOr& result) +Thread::WaitBlocker::WaitBlocker(int wait_options, Variant, NonnullRefPtr> waitee, KResultOr& result) : m_wait_options(wait_options) - , m_id_type(id_type) - , m_waitee_id(id) , m_result(result) + , m_waitee(move(waitee)) { } bool Thread::WaitBlocker::setup_blocker() { - switch (m_id_type) { - case P_PID: { - m_waitee = Process::from_pid(m_waitee_id); - if (!m_waitee || m_waitee->ppid() != Process::current().pid()) { - m_result = ECHILD; - m_error = true; - } - break; - } - case P_PGID: { - m_waitee_group = ProcessGroup::from_pgid(m_waitee_id); - if (!m_waitee_group) { - m_result = ECHILD; - m_error = true; - } - break; - } - case P_ALL: - break; - default: - VERIFY_NOT_REACHED(); - } - - if (m_error) - return false; - if (m_wait_options & WNOHANG) return false; - - // NOTE: unblock may be called within add_to_blocker_set, in which - // case it means that we already have a match without having to block. - // In that case add_to_blocker_set will return false. return add_to_blocker_set(Process::current().wait_blocker_set()); } void Thread::WaitBlocker::will_unblock_immediately_without_blocking(UnblockImmediatelyReason) { - if (!m_error) - Process::current().wait_blocker_set().try_unblock(*this); + Process::current().wait_blocker_set().try_unblock(*this); } void Thread::WaitBlocker::was_unblocked(bool) @@ -700,26 +668,20 @@ bool Thread::WaitBlocker::unblock(Process& process, UnblockFlags flags, u8 signa { VERIFY(flags != UnblockFlags::Terminated || signal == 0); // signal argument should be ignored for Terminated - switch (m_id_type) { - case P_PID: - VERIFY(m_waitee); - if (process.pid() != m_waitee_id) - return false; - break; - case P_PGID: - VERIFY(m_waitee_group); - if (process.pgid() != m_waitee_group->pgid()) - return false; - break; - case P_ALL: - if (flags == UnblockFlags::Disowned) { + bool do_not_unblock = m_waitee.visit( + [&](NonnullRefPtr const& waitee_process) { + return &process != waitee_process; + }, + [&](NonnullRefPtr const& waitee_process_group) { + return waitee_process_group->pgid() != process.pgid(); + }, + [&](Empty const&) { // Generic waiter won't be unblocked by disown - return false; - } - break; - default: - VERIFY_NOT_REACHED(); - } + return flags == UnblockFlags::Disowned; + }); + + if (do_not_unblock) + return false; switch (flags) { case UnblockFlags::Terminated: