Get rid of the undertaker and have waitpid() be the reaper.

For dead orphans, the scheduler calls reap().

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<Process>* g_processes;
-InlineLinkedList<Process>* 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<Process>;
-    g_dead_processes = new InlineLinkedList<Process>;
     s_hostname = new String("birx");
     Scheduler::initialize();
 }
@@ -286,8 +285,7 @@ int Process::exec(const String& path, Vector<String>&& arguments, Vector<String>
     }
 
     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<bool(Process&)> 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;

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<Process>* g_processes;
-extern InlineLinkedList<Process>* g_dead_processes;

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()

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();

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();

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?