From 62bd9bf9014d770b90b6c887698624059960d9e9 Mon Sep 17 00:00:00 2001 From: Jonas 'Sortie' Termansen Date: Tue, 17 Jul 2018 16:25:58 +0200 Subject: [PATCH] Fix pid 1 deadlocking when exiting with children. The child processes of pid 1 were being reparented to pid 1, causing an infinite loop. This change fixes the problem by adding a hook that runs in the last thread about to exit in a process. When pid 1 exits, the hook will prevent more processes and threads from being created, and then broadcast kill all processes and threads. The hook is not run in LastPrayer(), as that function runs in a worker thread and it can't block waiting for another thread to run LastPrayer() in the same thread. --- kernel/include/sortix/kernel/process.h | 4 +- kernel/kernel.cpp | 1 + kernel/kthread.cpp | 15 +++++ kernel/process.cpp | 83 +++++++++++++++++++++++--- kernel/thread.cpp | 12 +++- 5 files changed, 103 insertions(+), 12 deletions(-) diff --git a/kernel/include/sortix/kernel/process.h b/kernel/include/sortix/kernel/process.h index 2c9135ee..089c1942 100644 --- a/kernel/include/sortix/kernel/process.h +++ b/kernel/include/sortix/kernel/process.h @@ -135,6 +135,7 @@ public: public: Thread* firstthread; kthread_mutex_t threadlock; + size_t threads_not_exiting_count; bool threads_exiting; public: @@ -177,7 +178,6 @@ public: Process* Fork(); private: - void OnLastThreadExit(); void LastPrayer(); void WaitedFor(); void NotifyChildExit(Process* child, bool zombify); @@ -185,6 +185,8 @@ private: bool IsLimboDone(); public: + void OnLastThreadExit(); + void AfterLastThreadExit(); void ResetForExecute(); }; diff --git a/kernel/kernel.cpp b/kernel/kernel.cpp index 62891a07..670b09c5 100644 --- a/kernel/kernel.cpp +++ b/kernel/kernel.cpp @@ -380,6 +380,7 @@ extern "C" void KernelInit(unsigned long magic, multiboot_info_t* bootinfo_p) idlethread->kernelstacksize = STACK_SIZE; idlethread->kernelstackmalloced = false; system->firstthread = idlethread; + system->threads_not_exiting_count = 1; Scheduler::SetIdleThread(idlethread); // Let's create a regular kernel thread that can decide what happens next. diff --git a/kernel/kthread.cpp b/kernel/kthread.cpp index ac601561..cb8783be 100644 --- a/kernel/kthread.cpp +++ b/kernel/kthread.cpp @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -39,6 +40,20 @@ static void kthread_do_kill_thread(void* user) extern "C" void kthread_exit() { + Process* process = CurrentProcess(); + // Note: This requires all threads in this process to have been made by + // only threads in this process, except the initial thread. Otherwise more + // threads may appear, and we can't conclude whether this is the last thread + // in the process to exit. + kthread_mutex_lock(&process->threadlock); + bool is_last_to_exit = --process->threads_not_exiting_count == 0; + kthread_mutex_unlock(&process->threadlock); + // All other threads in the process have committed to exiting, though they + // might not have exited yet. However, we know they are only running the + // below code that schedules thread termination. It's therefore safe to run + // a final process termination step without interference. + if ( is_last_to_exit ) + process->OnLastThreadExit(); Worker::Schedule(kthread_do_kill_thread, CurrentThread()); Scheduler::ExitThread(); __builtin_unreachable(); diff --git a/kernel/process.cpp b/kernel/process.cpp index 69730c24..2f5301c9 100644 --- a/kernel/process.cpp +++ b/kernel/process.cpp @@ -71,6 +71,10 @@ namespace Sortix { kthread_mutex_t process_family_lock = KTHREAD_MUTEX_INITIALIZER; +// The system is shutting down and creation of additional processes and threads +// should be prevented. Protected by process_family_lock. +static bool is_init_exiting = false; + Process::Process() { program_image_path = NULL; @@ -134,6 +138,7 @@ Process::Process() firstthread = NULL; threadlock = KTHREAD_MUTEX_INITIALIZER; + threads_not_exiting_count = 0; threads_exiting = false; segments = NULL; @@ -171,6 +176,7 @@ Process::~Process() // process_family_lock taken assert(!mtable); assert(!cwd); assert(!root); + assert(!threads_not_exiting_count); assert(ptable); ptable->Free(pid); @@ -196,7 +202,35 @@ void Process::BootstrapDirectories(Ref root) this->cwd = root; } -void Process__OnLastThreadExit(void* user); +void Process::OnLastThreadExit() +{ + Process* init = Scheduler::GetInitProcess(); + assert(init); + + // Child processes can't be reparented away if we're init. The system is + // about to shut down, so broadcast SIGKILL every process and wait for every + // single process to exit. The operating system is finished when init has + // exited. + if ( init == this ) + { + ScopedLock lock(&process_family_lock); + // Forbid any more processes and threads from being created, so this + // loop will always terminate. + is_init_exiting = true; + kthread_mutex_lock(&ptrlock); + for ( pid_t pid = ptable->Next(0); 0 < pid; pid = ptable->Next(pid) ) + { + Process* process = ptable->Get(pid); + if ( process->pid != 0 && process != init ) + process->DeliverSignal(SIGKILL); + } + kthread_mutex_unlock(&ptrlock); + // NotifyChildExit always signals zombiecond for init when + // is_init_exiting is true. + while ( firstchild ) + kthread_cond_wait(&zombiecond, &process_family_lock); + } +} void Process::OnThreadDestruction(Thread* thread) { @@ -220,11 +254,16 @@ void Process::OnThreadDestruction(Thread* thread) ScheduleDeath(); } +void Process__AfterLastThreadExit(void* user) +{ + return ((Process*) user)->AfterLastThreadExit(); +} + void Process::ScheduleDeath() { // All our threads must have exited at this point. assert(!firstthread); - Worker::Schedule(Process__OnLastThreadExit, this); + Worker::Schedule(Process__AfterLastThreadExit, this); } // Useful for killing a partially constructed process without waiting for @@ -236,12 +275,7 @@ void Process::AbortConstruction() ScheduleDeath(); } -void Process__OnLastThreadExit(void* user) -{ - return ((Process*) user)->OnLastThreadExit(); -} - -void Process::OnLastThreadExit() +void Process::AfterLastThreadExit() { LastPrayer(); } @@ -258,6 +292,8 @@ void Process::DeleteTimers() } } +// This function runs in a worker thread and cannot block on another worker +// thread, or it may deadlock. void Process::LastPrayer() { assert(this); @@ -304,6 +340,16 @@ void Process::LastPrayer() // Init is nice and will gladly raise our orphaned children and zombies. Process* init = Scheduler::GetInitProcess(); assert(init); + + // Child processes can't be reparented away if we're init. OnLastThreadExit + // must have already killed all the child processes and prevented more from + // being created. + if ( init == this ) + { + assert(is_init_exiting); + assert(!firstchild); + } + while ( firstchild ) { Process* process = firstchild; @@ -436,7 +482,11 @@ void Process::NotifyChildExit(Process* child, bool zombify) zombiechild = child; } - if ( zombify ) + // Notify this parent process about the child exiting if it's meant to + // become a zombie process. Additionally, always notify init about children + // when init is exiting, because OnLastThreadExit needs to be able to catch + // every child exiting. + if ( zombify || (is_init_exiting && Scheduler::GetInitProcess() == this) ) { DeliverSignal(SIGCHLD); kthread_cond_broadcast(&zombiecond); @@ -651,6 +701,15 @@ Process* Process::Fork() kthread_mutex_lock(&process_family_lock); + // Forbid the creation of new processes if init has exited. + if ( is_init_exiting ) + { + kthread_mutex_unlock(&process_family_lock); + clone->AbortConstruction(); + return NULL; + } + + // Register the new process by assigning a pid. if ( (clone->pid = (clone->ptable = ptable)->Allocate(clone)) < 0 ) { kthread_mutex_unlock(&process_family_lock); @@ -1484,9 +1543,15 @@ pid_t sys_tfork(int flags, struct tfork* user_regs) #warning "You need to implement initializing the registers of the new thread" #endif + // Forbid the creation of new threads if init has exited. + ScopedLock process_family_lock_lock(&process_family_lock); + if ( is_init_exiting ) + return errno = EPERM, -1; + // If the thread could not be created, make the process commit suicide // in a manner such that we don't wait for its zombie. Thread* thread = CreateKernelThread(child_process, &cpuregs); + process_family_lock_lock.Reset(); if ( !thread ) { if ( making_process ) diff --git a/kernel/thread.cpp b/kernel/thread.cpp index 57874dfb..3f3fc4bd 100644 --- a/kernel/thread.cpp +++ b/kernel/thread.cpp @@ -125,14 +125,21 @@ Thread* CreateKernelThread(Process* process, struct thread_registers* regs) return errno = EINVAL, (Thread*) NULL; #endif + kthread_mutex_lock(&process->threadlock); + + // Note: Only allow the process itself to make threads, except the initial + // thread. This requirement is because kthread_exit() needs to know when + // it's the last thread in the process (using threads_not_exiting_count), + // and that no more threads will appear, so it can run some final process + // termination steps without any interference. + assert(!process->firstthread || process == CurrentProcess()); + Thread* thread = AllocateThread(); if ( !thread ) return NULL; memcpy(&thread->registers, regs, sizeof(struct thread_registers)); - kthread_mutex_lock(&process->threadlock); - // Create the family tree. thread->process = process; Thread* firsty = process->firstthread; @@ -140,6 +147,7 @@ Thread* CreateKernelThread(Process* process, struct thread_registers* regs) firsty->prevsibling = thread; thread->nextsibling = firsty; process->firstthread = thread; + process->threads_not_exiting_count++; kthread_mutex_unlock(&process->threadlock);