From 92e4e752178e173a3dd58718210430e21041fe19 Mon Sep 17 00:00:00 2001 From: Tony Wasserka Date: Tue, 25 Jul 2023 10:40:52 +0200 Subject: [PATCH] Merge DeferredSignalMutex.h and ScopedSignalMask.h into a single file Using a single file makes sense now that the individual files are much shorter and share common utility classes. --- FEXCore/Source/Interface/Context/Context.h | 2 +- FEXCore/Source/Interface/Core/Core.cpp | 2 +- .../Source/Utils/Allocator/64BitAllocator.cpp | 2 +- ...erredSignalMutex.h => SignalScopeGuards.h} | 30 +++++++- .../FEXHeaderUtils/ScopedSignalMask.h | 69 ------------------- .../Tools/FEXLoader/LinuxSyscalls/Syscalls.h | 2 +- .../LinuxSyscalls/SyscallsSMCTracking.cpp | 5 +- 7 files changed, 35 insertions(+), 77 deletions(-) rename FEXCore/include/FEXCore/Utils/{DeferredSignalMutex.h => SignalScopeGuards.h} (85%) delete mode 100644 FEXHeaderUtils/FEXHeaderUtils/ScopedSignalMask.h diff --git a/FEXCore/Source/Interface/Context/Context.h b/FEXCore/Source/Interface/Context/Context.h index 3b3572fce..109f0dbe6 100644 --- a/FEXCore/Source/Interface/Context/Context.h +++ b/FEXCore/Source/Interface/Context/Context.h @@ -14,8 +14,8 @@ #include #include #include -#include #include +#include #include #include #include diff --git a/FEXCore/Source/Interface/Core/Core.cpp b/FEXCore/Source/Interface/Core/Core.cpp index 0cc0b0602..b3cce811f 100644 --- a/FEXCore/Source/Interface/Core/Core.cpp +++ b/FEXCore/Source/Interface/Core/Core.cpp @@ -9,7 +9,6 @@ $end_info$ */ #include -#include "FEXCore/Utils/DeferredSignalMutex.h" #include "Interface/Context/Context.h" #include "Interface/Core/LookupCache.h" #include "Interface/Core/CPUID.h" @@ -45,6 +44,7 @@ $end_info$ #include #include #include +#include "FEXCore/Utils/SignalScopeGuards.h" #include #include #include diff --git a/FEXCore/Source/Utils/Allocator/64BitAllocator.cpp b/FEXCore/Source/Utils/Allocator/64BitAllocator.cpp index 2116b45ce..9c2d27133 100644 --- a/FEXCore/Source/Utils/Allocator/64BitAllocator.cpp +++ b/FEXCore/Source/Utils/Allocator/64BitAllocator.cpp @@ -5,8 +5,8 @@ #include #include #include +#include #include -#include #include #include #include diff --git a/FEXCore/include/FEXCore/Utils/DeferredSignalMutex.h b/FEXCore/include/FEXCore/Utils/SignalScopeGuards.h similarity index 85% rename from FEXCore/include/FEXCore/Utils/DeferredSignalMutex.h rename to FEXCore/include/FEXCore/Utils/SignalScopeGuards.h index f250edefb..5e1e86c9b 100644 --- a/FEXCore/include/FEXCore/Utils/DeferredSignalMutex.h +++ b/FEXCore/include/FEXCore/Utils/SignalScopeGuards.h @@ -111,6 +111,7 @@ namespace FEXCore { }; #endif + // Helper class to manage deferred signal refcounting within a block scope class DeferredSignalRefCountGuard final { public: explicit DeferredSignalRefCountGuard(FEXCore::Core::InternalThreadState *Thread) : Thread(Thread) { @@ -148,7 +149,7 @@ namespace FEXCore { }; #ifndef _WIN32 - // TODO: Duplicated, unify with ScopedSignalMask + // Helper class to mask POSIX signals within a block scope class ScopedSignalMasker final { public: explicit ScopedSignalMasker(uint64_t Mask) : OriginalMask(0) { @@ -173,6 +174,33 @@ namespace FEXCore { }; #endif + /** + * @brief Produces a wrapper object around a scoped lock of the given mutex + * while ensuring POSIX signals are masked while the mutex is locked + * + * Use this to prevent reentrancy issues of C++ mutexes with certain signal handlers. + * Common examples of such issues are: + * - C++ mutexes not unlocking due to a signal handler calling longjmp from within a scope owning the mutex + * - The signal handler itself using a mutex that would be re-locked if the handler gets invoked + * again before unlocking + * + * Ownership of the returned object may be moved, but it is NOT SAFE to move across threads. + */ + template class LockType = std::unique_lock, typename MutexType> + [[nodiscard]] static auto MaskSignalsAndLockMutex(MutexType& mutex, uint64_t Mask = ~0ULL) { +#ifndef _WIN32 + // Signals are masked first, and then the lock is acquired + struct { + ScopedSignalMasker mask; + LockType lock; + } scope_guard { ScopedSignalMasker { Mask }, LockType { mutex } }; + return scope_guard; +#else + // TODO: Doesn't block signals which may or may not cause issues. + return LockType { mutex }; +#endif + } + /** * @brief Produces a wrapper object around a scoped lock of the given mutex * while bumping the Thread's deferred signal refcount while the mutex is diff --git a/FEXHeaderUtils/FEXHeaderUtils/ScopedSignalMask.h b/FEXHeaderUtils/FEXHeaderUtils/ScopedSignalMask.h deleted file mode 100644 index 74f66bd36..000000000 --- a/FEXHeaderUtils/FEXHeaderUtils/ScopedSignalMask.h +++ /dev/null @@ -1,69 +0,0 @@ -// SPDX-License-Identifier: MIT -#pragma once - -#include -#include -#include -#include -#ifndef _WIN32 -#include -#include -#endif -#include - -namespace FHU { -#ifndef _WIN32 - /** - * Masks POSIX signals for the scope the object is active in - */ - class ScopedSignalMasker final { - public: - explicit ScopedSignalMasker(uint64_t Mask) : OriginalMask(0) { - // Mask all signals, storing the original incoming mask - ::syscall(SYS_rt_sigprocmask, SIG_SETMASK, &Mask, &*OriginalMask, sizeof(*OriginalMask)); - } - - // Move-only type - ScopedSignalMasker(const ScopedSignalMasker&) = delete; - ScopedSignalMasker& operator=(ScopedSignalMasker&) = delete; - ScopedSignalMasker(ScopedSignalMasker&& rhs) : OriginalMask(rhs.OriginalMask) { - rhs.OriginalMask.reset(); - } - - ~ScopedSignalMasker() { - if (OriginalMask) { - ::syscall(SYS_rt_sigprocmask, SIG_SETMASK, &OriginalMask, nullptr, sizeof(*OriginalMask)); - } - } - private: - std::optional OriginalMask{}; - }; -#endif - - /** - * @brief Produces a wrapper object around a scoped lock of the given mutex - * while ensuring POSIX signals are masked while the mutex is locked - * - * Use this to prevent reentrancy issues of C++ mutexes with certain signal handlers. - * Common examples of such issues are: - * - C++ mutexes not unlocking due to a signal handler calling longjmp from within a scope owning the mutex - * - The signal handler itself using a mutex that would be re-locked if the handler gets invoked - * again before unlocking - * - * Ownership of the returned object may be moved, but it is NOT SAFE to move across threads. - */ - template class LockType = std::unique_lock, typename MutexType> - [[nodiscard]] static auto MaskSignalsAndLockMutex(MutexType& mutex, uint64_t Mask = ~0ULL) { -#ifndef _WIN32 - // Signals are masked first, and then the lock is acquired - struct { - ScopedSignalMasker mask; - LockType lock; - } scope_guard { ScopedSignalMasker { Mask }, LockType { mutex } }; - return scope_guard; -#else - // TODO: Doesn't block signals which may or may not cause issues. - return LockType { mutex }; -#endif - } -} diff --git a/Source/Tools/FEXLoader/LinuxSyscalls/Syscalls.h b/Source/Tools/FEXLoader/LinuxSyscalls/Syscalls.h index a641b8a4d..3da407147 100644 --- a/Source/Tools/FEXLoader/LinuxSyscalls/Syscalls.h +++ b/Source/Tools/FEXLoader/LinuxSyscalls/Syscalls.h @@ -16,7 +16,7 @@ $end_info$ #include #include #include -#include +#include #include #include #include diff --git a/Source/Tools/FEXLoader/LinuxSyscalls/SyscallsSMCTracking.cpp b/Source/Tools/FEXLoader/LinuxSyscalls/SyscallsSMCTracking.cpp index b3b92dea4..e968f3987 100644 --- a/Source/Tools/FEXLoader/LinuxSyscalls/SyscallsSMCTracking.cpp +++ b/Source/Tools/FEXLoader/LinuxSyscalls/SyscallsSMCTracking.cpp @@ -16,11 +16,10 @@ $end_info$ #include "LinuxSyscalls/Syscalls.h" #include -#include #include #include #include -#include +#include namespace FEX::HLE { @@ -55,7 +54,7 @@ bool SyscallHandler::HandleSegfault(FEXCore::Core::InternalThreadState *Thread, { // Can't use the deferred signal lock in the SIGSEGV handler. - auto lk = FHU::MaskSignalsAndLockMutex(_SyscallHandler->VMATracking.Mutex); + auto lk = FEXCore::MaskSignalsAndLockMutex(_SyscallHandler->VMATracking.Mutex); auto VMATracking = &_SyscallHandler->VMATracking;