From 29163cfd0756cb467e9496a7ab32e2fa0696c951 Mon Sep 17 00:00:00 2001 From: Stefanos Kornilios Misis Poiitidis Date: Tue, 14 Jun 2022 22:28:07 +0300 Subject: [PATCH] Context: Fix CreateThread partial initialization issue --- .../Source/Interface/Context/Context.h | 5 ++-- .../FEXCore/Source/Interface/Core/Core.cpp | 24 +++++++------------ 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/External/FEXCore/Source/Interface/Context/Context.h b/External/FEXCore/Source/Interface/Context/Context.h index c71d33245..0ac38158f 100644 --- a/External/FEXCore/Source/Interface/Context/Context.h +++ b/External/FEXCore/Source/Interface/Context/Context.h @@ -113,7 +113,6 @@ namespace FEXCore::Context { FEXCore::HostFeatures HostFeatures; std::mutex ThreadCreationMutex; - uint64_t ThreadID{}; FEXCore::Core::InternalThreadState* ParentThread; std::vector Threads; std::atomic_bool CoreShuttingDown{false}; @@ -209,7 +208,7 @@ namespace FEXCore::Context { // Used for thread creation from syscalls /** - * @brief Used to create FEX thread objects in preparation for creating a true OS thread + * @brief Used to create FEX thread objects in preparation for creating a true OS thread. Does set a TID or PID. * * @param NewThreadState The initial thread state to setup for our state * @param ParentTID The PID that was the parent thread that created this @@ -232,7 +231,7 @@ namespace FEXCore::Context { FEXCore::Core::InternalThreadState* CreateThread(FEXCore::Core::CPUState *NewThreadState, uint64_t ParentTID); /** - * @brief Initializes the TLS data for a thread + * @brief Initializes TID, PID and TLS data for a thread * * @param Thread The internal FEX thread state object */ diff --git a/External/FEXCore/Source/Interface/Core/Core.cpp b/External/FEXCore/Source/Interface/Core/Core.cpp index b6530b65e..967061265 100644 --- a/External/FEXCore/Source/Interface/Core/Core.cpp +++ b/External/FEXCore/Source/Interface/Core/Core.cpp @@ -352,9 +352,6 @@ namespace FEXCore::Context { // Walk the threads and tell them to clear their caches // Useful when our block size is set to a large number and we need to step a single instruction for (auto &Thread : Threads) { - // Wait for thread to be fully constructed - // XXX: Look into thread partial construction issues - while(Thread->RunningEvents.WaitingToStart.load()) ; ClearCodeCache(Thread); } } @@ -553,14 +550,7 @@ namespace FEXCore::Context { } FEXCore::Core::InternalThreadState* Context::CreateThread(FEXCore::Core::CPUState *NewThreadState, uint64_t ParentTID) { - FEXCore::Core::InternalThreadState *Thread{}; - - // Grab the new thread object - { - std::lock_guard lk(ThreadCreationMutex); - Thread = Threads.emplace_back(new FEXCore::Core::InternalThreadState{}); - Thread->ThreadManager.TID = ++ThreadID; - } + FEXCore::Core::InternalThreadState *Thread = new FEXCore::Core::InternalThreadState{}; // Copy over the new thread state to the new object memcpy(Thread->CurrentFrame, NewThreadState, sizeof(FEXCore::Core::CPUState)); @@ -572,13 +562,19 @@ namespace FEXCore::Context { InitializeCompiler(Thread); InitializeThreadData(Thread); + // Insert after the Thread object has been fully initialized + { + std::lock_guard lk(ThreadCreationMutex); + Threads.push_back(Thread); + } + return Thread; } void Context::DestroyThread(FEXCore::Core::InternalThreadState *Thread) { // remove new thread object { - std::lock_guard lk(ThreadCreationMutex); + std::lock_guard lk(ThreadCreationMutex); auto It = std::find(Threads.begin(), Threads.end(), Thread); LOGMAN_THROW_A_FMT(It != Threads.end(), "Thread wasn't in Threads"); @@ -1135,9 +1131,7 @@ namespace FEXCore::Context { std::lock_guard lk(CTX->ThreadCreationMutex); for (auto &Thread : CTX->Threads) { - if (Thread->RunningEvents.Running.load()) { - InvalidateGuestThreadCodeRange(Thread, Start, Length); - } + InvalidateGuestThreadCodeRange(Thread, Start, Length); } }