From d90104f9e00906ee4144e43d33cfb6ad3a9cc38d Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Wed, 28 Nov 2018 22:01:24 +0100 Subject: [PATCH] Let reap() communicate the dead process's exit status to the caller. This way the scheduler doesn't need to plumb the exit status into the waiter. We still plumb the waitee pid though, I don't love it but it can be fixed. --- AK/Compiler.h | 2 +- Kernel/Process.cpp | 18 ++++++++++-------- Kernel/Process.h | 3 +-- Kernel/Scheduler.cpp | 9 ++++++--- 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/AK/Compiler.h b/AK/Compiler.h index 04fad92b7f..74abc1a4e8 100644 --- a/AK/Compiler.h +++ b/AK/Compiler.h @@ -7,4 +7,4 @@ #define NEVER_INLINE __attribute__ ((noinline)) #define MALLOC_ATTR __attribute__ ((malloc)) #define PURE __attribute__ ((pure)) - +#define WARN_UNUSED_RESULT __attribute__ ((warn_unused_result)) diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 30c8a40ad5..50686b0ab7 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -1411,13 +1411,15 @@ mode_t Process::sys$umask(mode_t mask) return old_mask; } -void Process::reap(Process& process) +int Process::reap(Process& process) { InterruptDisabler disabler; + int exit_status = (process.m_termination_status << 8) | process.m_termination_signal; dbgprintf("reap: %s(%u) {%s}\n", process.name().characters(), process.pid(), toString(process.state())); ASSERT(process.state() == Dead); g_processes->remove(&process); delete &process; + return exit_status; } pid_t Process::sys$waitpid(pid_t waitee, int* wstatus, int options) @@ -1429,6 +1431,9 @@ pid_t Process::sys$waitpid(pid_t waitee, int* wstatus, int options) if (!validate_write_typed(wstatus)) return -EFAULT; + int dummy_wstatus; + int& exit_status = wstatus ? *wstatus : dummy_wstatus; + { InterruptDisabler disabler; if (waitee != -1 && !Process::from_pid(waitee)) @@ -1439,10 +1444,10 @@ pid_t Process::sys$waitpid(pid_t waitee, int* wstatus, int options) if (waitee == -1) { pid_t reaped_pid = 0; InterruptDisabler disabler; - for_each_child([&reaped_pid] (Process& process) { + for_each_child([&reaped_pid, &exit_status] (Process& process) { if (process.state() == Dead) { reaped_pid = process.pid(); - reap(process); + exit_status = reap(process); } return true; }); @@ -1452,7 +1457,7 @@ pid_t Process::sys$waitpid(pid_t waitee, int* wstatus, int options) if (!waitee_process) return -ECHILD; if (waitee_process->state() == Dead) { - reap(*waitee_process); + exit_status = reap(*waitee_process); return waitee; } return 0; @@ -1460,7 +1465,6 @@ pid_t Process::sys$waitpid(pid_t waitee, int* wstatus, int options) } m_waitee = waitee; - m_waitee_status = 0; block(BlockedWait); sched_yield(); if (m_was_interrupted_while_blocked) @@ -1473,9 +1477,7 @@ pid_t Process::sys$waitpid(pid_t waitee, int* wstatus, int options) waitee_process = Process::from_pid(m_waitee); } ASSERT(waitee_process); - reap(*waitee_process); - if (wstatus) - *wstatus = m_waitee_status; + exit_status = reap(*waitee_process); return m_waitee; } diff --git a/Kernel/Process.h b/Kernel/Process.h index 2a03dcd400..029a1431b7 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -177,7 +177,7 @@ public: static void initialize(); void crash() NORETURN; - static void reap(Process&); + static int reap(Process&) WARN_UNUSED_RESULT; const TTY* tty() const { return m_tty; } @@ -267,7 +267,6 @@ private: void* m_kernelStack { nullptr }; dword m_timesScheduled { 0 }; pid_t m_waitee { -1 }; - int m_waitee_status { 0 }; int m_fdBlockedOnRead { -1 }; int m_blocked_fd { -1 }; size_t m_max_open_file_descriptors { 16 }; diff --git a/Kernel/Scheduler.cpp b/Kernel/Scheduler.cpp index 989ed0cfbb..c2471dfbe7 100644 --- a/Kernel/Scheduler.cpp +++ b/Kernel/Scheduler.cpp @@ -39,7 +39,6 @@ bool Scheduler::pick_next() if (child.state() != Process::Dead) return true; if (process.waitee() == -1 || process.waitee() == child.pid()) { - process.m_waitee_status = (child.m_termination_status << 8) | child.m_termination_signal; process.m_waitee = child.pid(); process.unblock(); return false; @@ -75,8 +74,12 @@ bool Scheduler::pick_next() } if (process.state() == Process::Dead) { - if (current != &process && !Process::from_pid(process.ppid())) - Process::reap(process); + if (current != &process && !Process::from_pid(process.ppid())) { + auto name = process.name(); + auto pid = process.pid(); + auto exit_status = Process::reap(process); + kprintf("reaped unparented process %s(%u), exit status: %u\n", name.characters(), pid, exit_status); + } return true; }