From 83b86d915ab27be68c8428761c8510817991b5ff Mon Sep 17 00:00:00 2001 From: lat9nq Date: Fri, 1 Apr 2022 18:29:08 -0400 Subject: k_scheduler_lock: Fix data race TSan reports a race between the main thread and T37 during IsLockedByCurrentThread and when it's set at the end of Lock(), respectively. Set owner_thread to an atomic pointer to fix it. Co-authored-by: bunnei --- src/core/hle/kernel/k_scheduler_lock.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/hle/kernel/k_scheduler_lock.h b/src/core/hle/kernel/k_scheduler_lock.h index 93c47f1b1..016e0a818 100644 --- a/src/core/hle/kernel/k_scheduler_lock.h +++ b/src/core/hle/kernel/k_scheduler_lock.h @@ -4,6 +4,7 @@ #pragma once +#include #include "common/assert.h" #include "core/hle/kernel/k_spin_lock.h" #include "core/hle/kernel/k_thread.h" @@ -75,7 +76,7 @@ private: KernelCore& kernel; KAlignedSpinLock spin_lock{}; s32 lock_count{}; - KThread* owner_thread{}; + std::atomic owner_thread{}; }; } // namespace Kernel -- cgit v1.2.3 From 5b5a1b7fa7470bd32b5481ae6a6cf5f4b07c80c8 Mon Sep 17 00:00:00 2001 From: lat9nq Date: Fri, 1 Apr 2022 18:32:20 -0400 Subject: kernel: Fix current_process race TSan reported a race at :258 and :803, so make current_process an atomic pointer. --- src/core/hle/kernel/kernel.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/hle/kernel/kernel.cpp b/src/core/hle/kernel/kernel.cpp index 34da7c23b..caa91a509 100644 --- a/src/core/hle/kernel/kernel.cpp +++ b/src/core/hle/kernel/kernel.cpp @@ -84,7 +84,7 @@ struct KernelCore::Impl { void InitializeCores() { for (u32 core_id = 0; core_id < Core::Hardware::NUM_CPU_CORES; core_id++) { - cores[core_id].Initialize(current_process->Is64BitProcess()); + cores[core_id].Initialize((*current_process).Is64BitProcess()); system.Memory().SetCurrentPageTable(*current_process, core_id); } } @@ -167,11 +167,11 @@ struct KernelCore::Impl { // Shutdown all processes. if (current_process) { - current_process->Finalize(); + (*current_process).Finalize(); // current_process->Close(); // TODO: The current process should be destroyed based on accurate ref counting after // calling Close(). Adding a manual Destroy() call instead to avoid a memory leak. - current_process->Destroy(); + (*current_process).Destroy(); current_process = nullptr; } @@ -697,7 +697,7 @@ struct KernelCore::Impl { // Lists all processes that exist in the current session. std::vector process_list; - KProcess* current_process{}; + std::atomic current_process{}; std::unique_ptr global_scheduler_context; Kernel::TimeManager time_manager; -- cgit v1.2.3 From d6a0666268d5ef51936c2475d33ab37ff14462e4 Mon Sep 17 00:00:00 2001 From: lat9nq Date: Fri, 1 Apr 2022 18:40:52 -0400 Subject: k_process: Fix data race TSan reported a race between thread 36 and thread 34, a read at :225 and a write at :225 respectively. Make total_proces_running_time_ticks atomic to avoid this race. --- src/core/hle/kernel/k_process.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/hle/kernel/k_process.h b/src/core/hle/kernel/k_process.h index 48b17fc74..9f171e3da 100644 --- a/src/core/hle/kernel/k_process.h +++ b/src/core/hle/kernel/k_process.h @@ -422,7 +422,7 @@ private: bool is_64bit_process = true; /// Total running time for the process in ticks. - u64 total_process_running_time_ticks = 0; + std::atomic total_process_running_time_ticks = 0; /// Per-process handle table for storing created object handles in. KHandleTable handle_table; -- cgit v1.2.3 From 6bcbbb29e7822f5ebd6ac985066dfdd0890492f0 Mon Sep 17 00:00:00 2001 From: lat9nq Date: Fri, 1 Apr 2022 18:57:40 -0400 Subject: k_thread: Fix data race TSan reports a data race between writing at cpp:1162 and reading at h:262. Make the thread_state atomic to prevent this. --- src/core/hle/kernel/k_thread.cpp | 4 ++-- src/core/hle/kernel/k_thread.h | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/core/hle/kernel/k_thread.cpp b/src/core/hle/kernel/k_thread.cpp index 94c8faf68..d3bb1c871 100644 --- a/src/core/hle/kernel/k_thread.cpp +++ b/src/core/hle/kernel/k_thread.cpp @@ -723,7 +723,7 @@ void KThread::UpdateState() { ASSERT(kernel.GlobalSchedulerContext().IsLocked()); // Set our suspend flags in state. - const auto old_state = thread_state; + const ThreadState old_state = thread_state; const auto new_state = static_cast(this->GetSuspendFlags()) | (old_state & ThreadState::Mask); thread_state = new_state; @@ -738,7 +738,7 @@ void KThread::Continue() { ASSERT(kernel.GlobalSchedulerContext().IsLocked()); // Clear our suspend flags in state. - const auto old_state = thread_state; + const ThreadState old_state = thread_state; thread_state = old_state & ThreadState::Mask; // Note the state change in scheduler. diff --git a/src/core/hle/kernel/k_thread.h b/src/core/hle/kernel/k_thread.h index f46db7298..d0fd85130 100644 --- a/src/core/hle/kernel/k_thread.h +++ b/src/core/hle/kernel/k_thread.h @@ -5,6 +5,7 @@ #pragma once #include +#include #include #include #include @@ -751,7 +752,7 @@ private: KAffinityMask original_physical_affinity_mask{}; s32 original_physical_ideal_core_id{}; s32 num_core_migration_disables{}; - ThreadState thread_state{}; + std::atomic thread_state{}; std::atomic termination_requested{}; bool wait_cancelled{}; bool cancellable{}; -- cgit v1.2.3 From 983916e9193a65d2cbd55039cc1569c46a7081c1 Mon Sep 17 00:00:00 2001 From: lat9nq Date: Fri, 1 Apr 2022 19:54:35 -0400 Subject: k_auto_object: Fix data race Change the memory order to acqure-release when we decrement the reference count. Prevents a race with line 89 reported by TSan. --- src/core/hle/kernel/k_auto_object.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/hle/kernel/k_auto_object.h b/src/core/hle/kernel/k_auto_object.h index 05779f2d5..abdb8ae7c 100644 --- a/src/core/hle/kernel/k_auto_object.h +++ b/src/core/hle/kernel/k_auto_object.h @@ -163,7 +163,7 @@ public: do { ASSERT(cur_ref_count > 0); } while (!m_ref_count.compare_exchange_weak(cur_ref_count, cur_ref_count - 1, - std::memory_order_relaxed)); + std::memory_order_acq_rel)); // If ref count hits zero, destroy the object. if (cur_ref_count - 1 == 0) { -- cgit v1.2.3 From b976cac49dd7bbfc461802d64d467f9c5b73ce3b Mon Sep 17 00:00:00 2001 From: lat9nq Date: Tue, 5 Apr 2022 00:37:46 -0400 Subject: k_system_control: Fix data race `return distribution(gen)` is a data race between a read and a write in two threads, reported by TSan. Remove static random number generators so they aren't using the same generator. --- src/core/hle/kernel/board/nintendo/nx/k_system_control.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/hle/kernel/board/nintendo/nx/k_system_control.cpp b/src/core/hle/kernel/board/nintendo/nx/k_system_control.cpp index 8027bec00..7765e7848 100644 --- a/src/core/hle/kernel/board/nintendo/nx/k_system_control.cpp +++ b/src/core/hle/kernel/board/nintendo/nx/k_system_control.cpp @@ -148,9 +148,9 @@ u64 GenerateUniformRange(u64 min, u64 max, F f) { } // Anonymous namespace u64 KSystemControl::GenerateRandomU64() { - static std::random_device device; - static std::mt19937 gen(device()); - static std::uniform_int_distribution distribution(1, std::numeric_limits::max()); + std::random_device device; + std::mt19937 gen(device()); + std::uniform_int_distribution distribution(1, std::numeric_limits::max()); return distribution(gen); } -- cgit v1.2.3