diff --git a/zion/object/process.cpp b/zion/object/process.cpp index 03a934d..56e72e9 100644 --- a/zion/object/process.cpp +++ b/zion/object/process.cpp @@ -47,16 +47,6 @@ glcr::RefPtr Process::GetThread(uint64_t tid) { return threads_[tid]; } -void Process::CheckState() { - MutexHolder lock(mutex_); - for (uint64_t i = 0; i < threads_.size(); i++) { - if (!threads_[i]->IsDying()) { - return; - } - } - Exit(); -} - glcr::RefPtr Process::ReleaseCapability(uint64_t cid) { return caps_.ReleaseCapability(cid); } @@ -72,24 +62,46 @@ uint64_t Process::AddExistingCapability(const glcr::RefPtr& cap) { void Process::Exit() { // TODO: Check this state elsewhere to ensure that we don't for instance // create a running thread on a finished process. - state_ = FINISHED; + state_ = CLEANUP; for (uint64_t i = 0; i < threads_.size(); i++) { if (!threads_[i]->IsDying()) { - threads_[i]->Cleanup(); + threads_[i]->SetState(Thread::CLEANUP); } } - // From this point onward no threads should be able to reach userspace. + gProcMan->CleanupProcess(id_); - // TODO: Unmap all userspace mappings. - // TODO: Clear capabilities. - - // TODO: In the future consider removing this from the process manager. - // I need to think through the implications because the process object - // will be kept alive by the process that created it most likely. + // Technically we may get interrupted here the cleanup process may start, + // truthfully that is fine. Once each thread is flagged for cleanup then it + // will no longer be scheduled again or need to be. if (gScheduler->CurrentProcess().id_ == id_) { gScheduler->Yield(); } } + +void Process::Cleanup() { + if (gScheduler->CurrentProcess().id_ == id_) { + panic("Can't clean up process from itself."); + } + if (state_ != CLEANUP) { + dbgln("WARN: Cleaning up process with non-cleanup state {}", + (uint64_t)state_); + state_ = CLEANUP; + } + + // 1. For each thread, call cleanup. + for (uint64_t i = 0; i < threads_.size(); i++) { + if (threads_[i]->GetState() == Thread::CLEANUP) { + threads_[i]->Cleanup(); + } + } + + // 2. Release all capabailities. TODO + + // 3. Unmap all user memory. TODO + + // 4. Release paging structures. TODO + state_ = FINISHED; +} diff --git a/zion/object/process.h b/zion/object/process.h index dd213af..45555aa 100644 --- a/zion/object/process.h +++ b/zion/object/process.h @@ -31,6 +31,7 @@ class Process : public KernelObject { UNSPECIFIED, SETUP, RUNNING, + CLEANUP, FINISHED, }; static glcr::RefPtr RootProcess(); @@ -55,14 +56,16 @@ class Process : public KernelObject { } uint64_t AddExistingCapability(const glcr::RefPtr& cap); - // Checks the state of all child threads and transitions to - // finished if all have finished. - void CheckState(); - State GetState() { return state_; } + // This stops all of the processes threads (they will no longer be scheduled) + // and flags the process for cleanup. void Exit(); + // This *should not* be called from a thread that belongs to this process. + // Rather it should be called from the cleanup thread. + void Cleanup(); + private: friend class glcr::MakeRefCountedFriend; Process(); diff --git a/zion/object/thread.cpp b/zion/object/thread.cpp index 0b69547..76d5acd 100644 --- a/zion/object/thread.cpp +++ b/zion/object/thread.cpp @@ -81,6 +81,12 @@ void Thread::Init() { SetRsp0(rsp0_start_); jump_user_space(rip_, rsp_, arg1_, arg2_); } +void Thread::SetState(State state) { + if (IsDying()) { + panic("Cannot set state on dying thread."); + } + state_ = state; +} void Thread::Exit() { #if K_THREAD_DEBUG @@ -92,23 +98,34 @@ void Thread::Exit() { curr_thread->tid(), pid(), tid()); } gProcMan->CleanupThread(curr_thread); - Cleanup(); - process_.CheckState(); + state_ = CLEANUP; gScheduler->Yield(); } void Thread::Cleanup() { - state_ = CLEANUP; + if (gScheduler->CurrentThread().get() == this) { + panic("Cannot cleanup thread from itself."); + } + if (state_ != CLEANUP) { + dbgln("WARN: Cleaning up thread with non-cleanup state {}", + (uint64_t)state_); + state_ = CLEANUP; + } + + // 1. Release User Stack + process_.vmas()->FreeUserStack(rsp_); + + // 2. Unblock waiting threads. while (blocked_threads_.size() != 0) { auto thread = blocked_threads_.PopFront(); thread->SetState(Thread::RUNNABLE); gScheduler->Enqueue(thread); } - state_ = FINISHED; - // TODO: Race condition when called from exit, once kernel stack manager - // actually reuses stacks this will cause an issue + + // 3. Release Kernel Stack KernelVmm::FreeKernelStack(rsp0_start_); - process_.vmas()->FreeUserStack(rsp_); + + state_ = FINISHED; } void Thread::Wait() { diff --git a/zion/object/thread.h b/zion/object/thread.h index 1d96b21..fcde8a8 100644 --- a/zion/object/thread.h +++ b/zion/object/thread.h @@ -55,7 +55,7 @@ class Thread : public KernelObject, public glcr::IntrusiveListNode { // State Management. State GetState() { return state_; }; - void SetState(State state) { state_ = state; } + void SetState(State state); bool IsDying() { return state_ == CLEANUP || state_ == FINISHED; } // Exits this thread. diff --git a/zion/scheduler/cleanup.cpp b/zion/scheduler/cleanup.cpp index cc4f9fb..3a9f421 100644 --- a/zion/scheduler/cleanup.cpp +++ b/zion/scheduler/cleanup.cpp @@ -8,11 +8,13 @@ void ProcessCleanup::CleanupLoop() { // TODO: I think we need to protect these lists with a mutex as well. while (!process_list_.empty()) { auto proc = process_list_.PopFront(); - dbgln("CLEANUP Proc {}", proc->id()); + dbgln("CLEANUP Process: {}", proc->id()); + proc->Cleanup(); } while (!thread_list_.empty()) { auto thread = thread_list_.PopFront(); - dbgln("CLEANUP Thread {}.{}", thread->pid(), thread->tid()); + dbgln("CLEANUP Thread: {}.{}", thread->pid(), thread->tid()); + thread->Cleanup(); } } } diff --git a/zion/syscall/process.cpp b/zion/syscall/process.cpp index 0d9926c..05048be 100644 --- a/zion/syscall/process.cpp +++ b/zion/syscall/process.cpp @@ -9,7 +9,6 @@ z_err_t ProcessExit(ZProcessExitReq* req) { auto curr_thread = gScheduler->CurrentThread(); dbgln("Exit code: {}", static_cast(req->code)); - gProcMan->CleanupProcess(curr_thread->pid()); curr_thread->process().Exit(); panic("Returned from thread exit"); return glcr::UNIMPLEMENTED;