From efb276f48902c8f228c464763309ac410c02b69f Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Sat, 30 Nov 2024 22:48:29 -0800 Subject: [PATCH] FEXCore: Removes ExitHandler and RunUntilExit Now that all the threading behaviour has been correctly separated/moved to the frontend, these functions serve no purpose. - Instead of using RunUntilExit, all threads can use `ExecuteThread` directly, since there's nothing special about the primary thread now. - This also removes the public function definition of `ExecutionThread` since that was only used for threading logic. - Instead of using an exit handler, just do the same cleanup after `ExecuteThread` has returned. - Just make gdbserver is cleaned up early if it exists since it may want to send some things to the connected gdb instance before threads are exited. --- FEXCore/Source/Interface/Context/Context.cpp | 8 ------ FEXCore/Source/Interface/Context/Context.h | 16 ++--------- FEXCore/Source/Interface/Core/Core.cpp | 28 ++++++------------- FEXCore/include/FEXCore/Core/Context.h | 14 ---------- Source/Tools/FEXLoader/FEXLoader.cpp | 10 +++---- .../LinuxSyscalls/GdbServer.cpp | 2 -- .../LinuxSyscalls/Syscalls/Thread.cpp | 4 +-- .../TestHarnessRunner/TestHarnessRunner.cpp | 2 +- 8 files changed, 18 insertions(+), 66 deletions(-) diff --git a/FEXCore/Source/Interface/Context/Context.cpp b/FEXCore/Source/Interface/Context/Context.cpp index 65ed7b9114..d62aa36b7a 100644 --- a/FEXCore/Source/Interface/Context/Context.cpp +++ b/FEXCore/Source/Interface/Context/Context.cpp @@ -24,14 +24,6 @@ fextl::unique_ptr FEXCore::Context::Context::CreateNe return fextl::make_unique(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 index ab5e6fd465..fccc2960d1 100644 --- 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 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 index 073130b3c2..a5d7ccc996 100644 --- 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(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(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 lk(Thread->LookupCache->WriteLock); diff --git a/FEXCore/include/FEXCore/Core/Context.h b/FEXCore/include/FEXCore/Core/Context.h index c6db1adeb1..d94518f837 100644 --- 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 index 053af847f4..60a8c49936 100644 --- 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 index 91fa1ef08c..335b161c90 100644 --- 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 index d074c241d5..f90a3bddca 100644 --- 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 index d922be9e38..11edc6453f 100644 --- 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.