[Zion] Move existing proc/thread cleanup calls to the cleanup thread.

zion/object/process.cpp

@@ -47,16 +47,6 @@ glcr::RefPtr<Thread> 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<Capability> Process::ReleaseCapability(uint64_t cid) {
   return caps_.ReleaseCapability(cid);
 }
@@ -72,24 +62,46 @@ uint64_t Process::AddExistingCapability(const glcr::RefPtr<Capability>& 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;
+}

zion/object/process.h

@@ -31,6 +31,7 @@ class Process : public KernelObject {
     UNSPECIFIED,
     SETUP,
     RUNNING,
+    CLEANUP,
     FINISHED,
   };
   static glcr::RefPtr<Process> RootProcess();
@@ -55,14 +56,16 @@ class Process : public KernelObject {
   }
   uint64_t AddExistingCapability(const glcr::RefPtr<Capability>& 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>;
   Process();

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

zion/object/thread.h

@@ -55,7 +55,7 @@ class Thread : public KernelObject, public glcr::IntrusiveListNode<Thread> {
 
   // 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.

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

zion/syscall/process.cpp

@@ -9,7 +9,6 @@
 z_err_t ProcessExit(ZProcessExitReq* req) {
   auto curr_thread = gScheduler->CurrentThread();
   dbgln("Exit code: {}", static_cast<glcr::ErrorCode>(req->code));
-  gProcMan->CleanupProcess(curr_thread->pid());
   curr_thread->process().Exit();
   panic("Returned from thread exit");
   return glcr::UNIMPLEMENTED;