diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 619195dcfa..3f308334ea 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -21,6 +21,7 @@ //#define DEBUG_IO //#define TASK_DEBUG //#define FORK_DEBUG +//#define SIGNAL_DEBUG #define MAX_PROCESS_GIDS 32 // FIXME: Only do a single validation for accesses that don't span multiple pages. @@ -43,7 +44,6 @@ static const DWORD defaultStackSize = 16384; static pid_t next_pid; InlineLinkedList* g_processes; -InlineLinkedList* g_dead_processes; static String* s_hostname; static String& hostnameStorage(InterruptDisabler&) @@ -67,7 +67,6 @@ void Process::initialize() #endif next_pid = 0; g_processes = new InlineLinkedList; - g_dead_processes = new InlineLinkedList; s_hostname = new String("birx"); Scheduler::initialize(); } @@ -286,8 +285,7 @@ int Process::exec(const String& path, Vector&& arguments, Vector } InterruptDisabler disabler; - if (current == this) - Scheduler::prepare_to_modify_own_tss(); + Scheduler::prepare_to_modify_tss(*this); m_name = parts.takeLast(); @@ -716,21 +714,20 @@ void Process::dispatch_signal(byte signal) return terminate_due_to_signal(signal); } - m_tss_to_resume_kernel = m_tss; -#ifdef SIGNAL_DEBUG - kprintf("resume tss pc: %w:%x\n", m_tss_to_resume_kernel.cs, m_tss_to_resume_kernel.eip); -#endif + Scheduler::prepare_to_modify_tss(*this); word ret_cs = m_tss.cs; dword ret_eip = m_tss.eip; dword ret_eflags = m_tss.eflags; bool interrupting_in_kernel = (ret_cs & 3) == 0; - - if ((ret_cs & 3) == 0) { - // FIXME: Handle send_signal to process currently in kernel code. + if (interrupting_in_kernel) { dbgprintf("dispatch_signal to %s(%u) in state=%s with return to %w:%x\n", name().characters(), pid(), toString(state()), ret_cs, ret_eip); ASSERT(is_blocked()); + m_tss_to_resume_kernel = m_tss; +#ifdef SIGNAL_DEBUG + dbgprintf("resume tss pc: %w:%x\n", m_tss_to_resume_kernel.cs, m_tss_to_resume_kernel.eip); +#endif } ProcessPagingScope pagingScope(*this); @@ -809,14 +806,14 @@ void Process::dispatch_signal(byte signal) m_pending_signals &= ~(1 << signal); #ifdef SIGNAL_DEBUG - dbgprintf("signal: Okay, %s(%u) has been primed\n", name().characters(), pid()); + dbgprintf("signal: Okay, %s(%u) {%s} has been primed with signal handler %w:%x\n", name().characters(), pid(), toString(state()), m_tss.cs, m_tss.eip); #endif } void Process::sys$sigreturn() { InterruptDisabler disabler; - Scheduler::prepare_to_modify_own_tss(); + Scheduler::prepare_to_modify_tss(*this); m_tss = m_tss_to_resume_kernel; #ifdef SIGNAL_DEBUG dbgprintf("sys$sigreturn in %s(%u)\n", name().characters(), pid()); @@ -846,19 +843,6 @@ void Process::crash() ASSERT_NOT_REACHED(); } -void Process::doHouseKeeping() -{ - if (g_dead_processes->isEmpty()) - return; - InterruptDisabler disabler; - Process* next = nullptr; - for (auto* deadProcess = g_dead_processes->head(); deadProcess; deadProcess = next) { - next = deadProcess->next(); - delete deadProcess; - } - g_dead_processes->clear(); -} - void Process::for_each(Function callback) { ASSERT_INTERRUPTS_DISABLED(); @@ -1226,20 +1210,34 @@ mode_t Process::sys$umask(mode_t mask) return old_mask; } +void Process::reap(pid_t pid) +{ + InterruptDisabler disabler; + auto* process = Process::from_pid(pid); + ASSERT(process); + dbgprintf("reap: %s(%u) {%s}\n", process->name().characters(), process->pid(), toString(process->state())); + ASSERT(process->state() == Dead); + g_processes->remove(process); + delete process; +} + pid_t Process::sys$waitpid(pid_t waitee, int* wstatus, int options) { if (wstatus) VALIDATE_USER_WRITE(wstatus, sizeof(int)); - InterruptDisabler disabler; - if (!Process::from_pid(waitee)) - return -1; + { + InterruptDisabler disabler; + if (!Process::from_pid(waitee)) + return -ECHILD; + } m_waitee = waitee; m_waitee_status = 0; block(BlockedWait); sched_yield(); if (m_was_interrupted_while_blocked) return -EINTR; + reap(waitee); if (wstatus) *wstatus = m_waitee_status; return m_waitee; diff --git a/Kernel/Process.h b/Kernel/Process.h index dd3dfc959d..13661def26 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -46,7 +46,6 @@ public: Skip1SchedulerPass, Skip0SchedulerPasses, Dead, - Forgiven, BeingInspected, BlockedSleep, BlockedWait, @@ -89,8 +88,6 @@ public: FileDescriptor* file_descriptor(int fd); const FileDescriptor* file_descriptor(int fd) const; - static void doHouseKeeping(); - void block(Process::State); void unblock(); @@ -163,6 +160,7 @@ public: static void initialize(); void crash() NORETURN; + static void reap(pid_t); const TTY* tty() const { return m_tty; } @@ -308,7 +306,6 @@ static inline const char* toString(Process::State state) case Process::Dead: return "Dead"; case Process::Skip1SchedulerPass: return "Skip1"; case Process::Skip0SchedulerPasses: return "Skip0"; - case Process::Forgiven: return "Forgiven"; case Process::BlockedSleep: return "Sleep"; case Process::BlockedWait: return "Wait"; case Process::BlockedRead: return "Read"; @@ -322,4 +319,3 @@ extern void block(Process::State); extern void sleep(DWORD ticks); extern InlineLinkedList* g_processes; -extern InlineLinkedList* g_dead_processes; diff --git a/Kernel/Scheduler.cpp b/Kernel/Scheduler.cpp index bbc0ee54de..a4227a6c3c 100644 --- a/Kernel/Scheduler.cpp +++ b/Kernel/Scheduler.cpp @@ -43,7 +43,6 @@ bool Scheduler::pick_next() if (waitee->state() == Process::Dead) { process.m_waitee_status = (waitee->m_termination_status << 8) | waitee->m_termination_signal; process.unblock(); - waitee->set_state(Process::Forgiven); } return true; } @@ -66,18 +65,11 @@ bool Scheduler::pick_next() return true; } - // Forgive dead orphans. if (process.state() == Process::Dead) { if (!Process::from_pid(process.ppid())) - process.set_state(Process::Forgiven); - } - - // Release the forgiven. - if (process.state() == Process::Forgiven) { - g_processes->remove(&process); - g_dead_processes->append(&process); + Process::reap(process.pid()); return true; - }; + } return true; }); @@ -241,11 +233,13 @@ void Scheduler::prepare_for_iret_to_new_process() load_task_register(s_redirection.selector); } -void Scheduler::prepare_to_modify_own_tss() +void Scheduler::prepare_to_modify_tss(Process& process) { - // This ensures that a process modifying its own TSS in order to yield() - // and end up somewhere else doesn't just end up right after the yield(). - load_task_register(s_redirection.selector); + // This ensures that a currently running process modifying its own TSS + // in order to yield() and end up somewhere else doesn't just end up + // right after the yield(). + if (current == &process) + load_task_register(s_redirection.selector); } static void hlt_loop() diff --git a/Kernel/Scheduler.h b/Kernel/Scheduler.h index 37682cc7c4..c4be384fe3 100644 --- a/Kernel/Scheduler.h +++ b/Kernel/Scheduler.h @@ -14,7 +14,7 @@ public: static bool yield(); static bool context_switch(Process&); static void prepare_for_iret_to_new_process(); - static void prepare_to_modify_own_tss(); + static void prepare_to_modify_tss(Process&); }; int sched_yield(); diff --git a/Kernel/init.cpp b/Kernel/init.cpp index aa17ed2470..0946276ee1 100644 --- a/Kernel/init.cpp +++ b/Kernel/init.cpp @@ -141,15 +141,6 @@ void dump_backtrace(bool use_ksyms) } #endif -static void undertaker_main() NORETURN; -static void undertaker_main() -{ - for (;;) { - Process::doHouseKeeping(); - sleep(300); - } -} - static void spawn_stress() NORETURN; static void spawn_stress() { @@ -306,7 +297,6 @@ void init() Process::initialize(); - Process::create_kernel_process(undertaker_main, "undertaker"); Process::create_kernel_process(init_stage2, "init_stage2"); Scheduler::pick_next(); diff --git a/Userland/sh.cpp b/Userland/sh.cpp index 039bada919..e3b40174f6 100644 --- a/Userland/sh.cpp +++ b/Userland/sh.cpp @@ -248,7 +248,14 @@ static int runcmd(char* cmd) } int wstatus = 0; - waitpid(child, &wstatus, 0); + int rc; + do { + rc = waitpid(child, &wstatus, 0); + if (rc < 0 && errno != EINTR) { + perror("waitpid"); + break; + } + } while(errno == EINTR); // FIXME: Should I really have to tcsetpgrp() after my child has exited? // Is the terminal controlling pgrp really still the PGID of the dead process?