Merge pull request #4188 from Sonicadvance1/fexcore_remove_unnecessary

FEXCore: Removes ExitHandler and RunUntilExit
FEX-Emu · Dec 1, 2024 · 41c8731 · 41c8731
2 parents 9febdde + efb276f
commit 41c8731
Show file tree

Hide file tree

Showing 8 changed files with 18 additions and 66 deletions.
diff --git a/FEXCore/Source/Interface/Context/Context.cpp b/FEXCore/Source/Interface/Context/Context.cpp
@@ -24,14 +24,6 @@ fextl::unique_ptr<FEXCore::Context::Context> FEXCore::Context::Context::CreateNe
   return fextl::make_unique<FEXCore::Context::ContextImpl>(Features);
 }
 
-void FEXCore::Context::ContextImpl::SetExitHandler(ExitHandler handler) {
-  CustomExitHandler = std::move(handler);
-}
-
-ExitHandler FEXCore::Context::ContextImpl::GetExitHandler() const {
-  return CustomExitHandler;
-}
-
 void FEXCore::Context::ContextImpl::CompileRIP(FEXCore::Core::InternalThreadState* Thread, uint64_t GuestRIP) {
   CompileBlock(Thread->CurrentFrame, GuestRIP);
 }

diff --git a/FEXCore/Source/Interface/Context/Context.h b/FEXCore/Source/Interface/Context/Context.h
@@ -81,11 +81,6 @@ class ContextImpl final : public FEXCore::Context::Context {
   // Context base class implementation.
   bool InitCore() override;
 
-  void SetExitHandler(ExitHandler handler) override;
-  ExitHandler GetExitHandler() const override;
-
-  void RunUntilExit(FEXCore::Core::InternalThreadState* Thread) override;
-
   void ExecuteThread(FEXCore::Core::InternalThreadState* Thread) override;
 
   void CompileRIP(FEXCore::Core::InternalThreadState* Thread, uint64_t GuestRIP) override;
@@ -113,15 +108,15 @@ class ContextImpl final : public FEXCore::Context::Context {
    * Usecases:
    *  Parent thread Creation:
    *    - Thread = CreateThread(InitialRIP, InitialStack, nullptr, 0);
-   *    - CTX->RunUntilExit(Thread);
+   *    - CTX->ExecuteThread(Thread);
    *  OS thread Creation:
    *    - Thread = CreateThread(0, 0, NewState, PPID);
    *    - Thread->ExecutionThread = FEXCore::Threads::Thread::Create(ThreadHandler, Arg);
-   *    - ThreadHandler calls `CTX->ExecutionThread(Thread)`
+   *    - ThreadHandler calls `CTX->ExecuteThread(Thread)`
    *  OS fork (New thread created with a clone of thread state):
    *    - clone{2, 3}
    *    - Thread = CreateThread(0, 0, CopyOfThreadState, PPID);
-   *    - ExecutionThread(Thread); // Starts executing without creating another host thread
+   *    - ExecuteThread(Thread); // Starts executing without creating another host thread
    *  Thunk callback executing guest code from native host thread
    *    - Thread = CreateThread(0, 0, NewState, PPID);
    *    - HandleCallback(Thread, RIP);
@@ -130,9 +125,6 @@ class ContextImpl final : public FEXCore::Context::Context {
   FEXCore::Core::InternalThreadState*
   CreateThread(uint64_t InitialRIP, uint64_t StackPointer, const FEXCore::Core::CPUState* NewThreadState, uint64_t ParentTID) override;
 
-  // Public for threading
-  void ExecutionThread(FEXCore::Core::InternalThreadState* Thread) override;
-
   /**
    * @brief Destroys this FEX thread object and stops tracking it internally
    *
@@ -246,8 +238,6 @@ class ContextImpl final : public FEXCore::Context::Context {
   FEXCore::ThunkHandler* ThunkHandler {};
   fextl::unique_ptr<FEXCore::CPU::Dispatcher> Dispatcher;
 
-  FEXCore::Context::ExitHandler CustomExitHandler;
-
   SignalDelegator* SignalDelegation {};
   X86GeneratedCode X86CodeGen;
 

diff --git a/FEXCore/Source/Interface/Core/Core.cpp b/FEXCore/Source/Interface/Core/Core.cpp
@@ -363,16 +363,17 @@ void ContextImpl::HandleCallback(FEXCore::Core::InternalThreadState* Thread, uin
   static_cast<ContextImpl*>(Thread->CTX)->Dispatcher->ExecuteJITCallback(Thread->CurrentFrame, RIP);
 }
 
-void ContextImpl::RunUntilExit(FEXCore::Core::InternalThreadState* Thread) {
-  ExecutionThread(Thread);
+void ContextImpl::ExecuteThread(FEXCore::Core::InternalThreadState* Thread) {
+  Dispatcher->ExecuteDispatch(Thread->CurrentFrame);
 
-  if (CustomExitHandler) {
-    CustomExitHandler(Thread);
+  {
+    // Ensure the Code Object Serialization service has fully serialized this thread's data before clearing the cache
+    // Use the thread's object cache ref counter for this
+    CodeSerialize::CodeObjectSerializeService::WaitForEmptyJobQueue(&Thread->ObjectCacheRefCounter);
   }
-}
 
-void ContextImpl::ExecuteThread(FEXCore::Core::InternalThreadState* Thread) {
-  Dispatcher->ExecuteDispatch(Thread->CurrentFrame);
+  // If it is the parent thread that died then just leave
+  FEX_TODO("This doesn't make sense when the parent thread doesn't outlive its children");
 }
 
 void ContextImpl::InitializeCompiler(FEXCore::Core::InternalThreadState* Thread) {
@@ -861,19 +862,6 @@ uintptr_t ContextImpl::CompileBlock(FEXCore::Core::CpuStateFrame* Frame, uint64_
   return (uintptr_t)CodePtr;
 }
 
-void ContextImpl::ExecutionThread(FEXCore::Core::InternalThreadState* Thread) {
-  static_cast<ContextImpl*>(Thread->CTX)->Dispatcher->ExecuteDispatch(Thread->CurrentFrame);
-
-  {
-    // Ensure the Code Object Serialization service has fully serialized this thread's data before clearing the cache
-    // Use the thread's object cache ref counter for this
-    CodeSerialize::CodeObjectSerializeService::WaitForEmptyJobQueue(&Thread->ObjectCacheRefCounter);
-  }
-
-  // If it is the parent thread that died then just leave
-  FEX_TODO("This doesn't make sense when the parent thread doesn't outlive its children");
-}
-
 static void InvalidateGuestThreadCodeRange(FEXCore::Core::InternalThreadState* Thread, uint64_t Start, uint64_t Length) {
   std::lock_guard<std::recursive_mutex> lk(Thread->LookupCache->WriteLock);
 

diff --git a/FEXCore/include/FEXCore/Core/Context.h b/FEXCore/include/FEXCore/Core/Context.h
@@ -94,19 +94,6 @@ class Context {
    */
   FEX_DEFAULT_VISIBILITY virtual bool InitCore() = 0;
 
-  FEX_DEFAULT_VISIBILITY virtual void SetExitHandler(ExitHandler handler) = 0;
-  FEX_DEFAULT_VISIBILITY virtual ExitHandler GetExitHandler() const = 0;
-
-  /**
-   * @brief Runs the CPU core until it exits
-   *
-   * If an Exit handler has been registered, this function won't return until the core
-   * has shutdown.
-   *
-   * @param CTX The context that we created
-   */
-  FEX_DEFAULT_VISIBILITY virtual void RunUntilExit(FEXCore::Core::InternalThreadState* Thread) = 0;
-
   /**
    * @brief Executes the supplied thread context on the current thread until a return is requested
    */
@@ -158,7 +145,6 @@ class Context {
   FEX_DEFAULT_VISIBILITY virtual FEXCore::Core::InternalThreadState* CreateThread(
     uint64_t InitialRIP, uint64_t StackPointer, const FEXCore::Core::CPUState* NewThreadState = nullptr, uint64_t ParentTID = 0) = 0;
 
-  FEX_DEFAULT_VISIBILITY virtual void ExecutionThread(FEXCore::Core::InternalThreadState* Thread) = 0;
   FEX_DEFAULT_VISIBILITY virtual void DestroyThread(FEXCore::Core::InternalThreadState* Thread) = 0;
 #ifndef _WIN32
   FEX_DEFAULT_VISIBILITY virtual void LockBeforeFork(FEXCore::Core::InternalThreadState* Thread) {}

diff --git a/Source/Tools/FEXLoader/FEXLoader.cpp b/Source/Tools/FEXLoader/FEXLoader.cpp
@@ -604,11 +604,6 @@ int main(int argc, char** argv, char** const envp) {
 
   SyscallHandler->DeserializeSeccompFD(ParentThread, FEXSeccompFD);
 
-  // There might already be an exit handler, leave it installed
-  if (!CTX->GetExitHandler()) {
-    CTX->SetExitHandler([&](FEXCore::Core::InternalThreadState* Thread) { SyscallHandler->TM.Stop(); });
-  }
-
   const bool AOTEnabled = AOTIRLoad() || AOTIRCapture() || AOTIRGenerate();
   if (AOTEnabled) {
     LogMan::Msg::IFmt("Warning: AOTIR is experimental, and might lead to crashes. "
@@ -646,9 +641,12 @@ int main(int argc, char** argv, char** const envp) {
       FEX::AOT::AOTGenSection(CTX.get(), Section);
     }
   } else {
-    CTX->RunUntilExit(ParentThread->Thread);
+    CTX->ExecuteThread(ParentThread->Thread);
   }
 
+  DebugServer.reset();
+  SyscallHandler->TM.Stop();
+
   if (AOTEnabled) {
     if (FHU::Filesystem::CreateDirectories(fextl::fmt::format("{}/aotir", FEXCore::Config::GetDataDirectory()))) {
       CTX->WriteFilesWithCode([](const fextl::string& fileid, const fextl::string& filename) {

diff --git a/Source/Tools/LinuxEmulation/LinuxSyscalls/GdbServer.cpp b/Source/Tools/LinuxEmulation/LinuxSyscalls/GdbServer.cpp
@@ -96,8 +96,6 @@ GdbServer::GdbServer(FEXCore::Context::Context* ctx, FEX::HLE::SignalDelegator*
   // Pass all signals by default
   std::fill(PassSignals.begin(), PassSignals.end(), true);
 
-  ctx->SetExitHandler([this](FEXCore::Core::InternalThreadState* Thread) { CoreShuttingDown = true; });
-
   // This is a total hack as there is currently no way to resume once hitting a segfault
   // But it's semi-useful for debugging.
   for (uint32_t Signal = 0; Signal <= FEX::HLE::SignalDelegator::MAX_SIGNALS; ++Signal) {

diff --git a/Source/Tools/LinuxEmulation/LinuxSyscalls/Syscalls/Thread.cpp b/Source/Tools/LinuxEmulation/LinuxSyscalls/Syscalls/Thread.cpp
@@ -73,7 +73,7 @@ static void* ThreadHandler(void* Data) {
   // Handler is a stack object on the parent thread, and will be invalid after notification.
   Handler->StartRunningResponse.NotifyOne();
 
-  CTX->ExecutionThread(Thread->Thread);
+  CTX->ExecuteThread(Thread->Thread);
   FEX::HLE::_SyscallHandler->UninstallTLSState(Thread);
   FEX::HLE::_SyscallHandler->TM.DestroyThread(Thread);
   return nullptr;
@@ -238,7 +238,7 @@ uint64_t HandleNewClone(FEX::HLE::ThreadStateObject* Thread, FEXCore::Context::C
 
   // Start exuting the thread directly
   // Our host clone starts in a new stack space, so it can't return back to the JIT space
-  CTX->ExecutionThread(Thread->Thread);
+  CTX->ExecuteThread(Thread->Thread);
 
   FEX::HLE::_SyscallHandler->UninstallTLSState(Thread);
 

diff --git a/Source/Tools/TestHarnessRunner/TestHarnessRunner.cpp b/Source/Tools/TestHarnessRunner/TestHarnessRunner.cpp
@@ -332,7 +332,7 @@ int main(int argc, char** argv, char** const envp) {
 
     int LongJumpVal = setjmp(LongJumpHandler::LongJump);
     if (!LongJumpVal) {
-      CTX->RunUntilExit(ParentThread->Thread);
+      CTX->ExecuteThread(ParentThread->Thread);
     }
 
     // Just re-use compare state. It also checks against the expected values in config.