From 5b3e8e1c5d756834a669ee4c820b24fb089b5e21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Lam?= Date: Mon, 2 Jul 2018 16:10:57 +0200 Subject: [PATCH] stratosphere: Use RAII for locks This renames the Mutex class member functions so that the mutex types satisfy Lockable. This makes them usable with standard std::scoped_lock and std::unique_lock, which lets us use RAII and avoids the need for a custom RAII wrapper :) --- stratosphere/fs_mitm/source/fsmitm_worker.cpp | 4 +- .../fs_mitm/source/mitm_query_service.cpp | 9 ++- .../include/stratosphere/hossynch.hpp | 12 ++-- .../source/multithreadedwaitablemanager.cpp | 10 ++-- .../source/waitablemanager.cpp | 7 +-- stratosphere/pm/source/pm_debug_monitor.cpp | 4 +- stratosphere/pm/source/pm_info.cpp | 2 +- stratosphere/pm/source/pm_process_wait.hpp | 15 ++--- stratosphere/pm/source/pm_registration.cpp | 58 ++++++------------- stratosphere/pm/source/pm_registration.hpp | 12 +--- stratosphere/pm/source/pm_shell.cpp | 10 ++-- 11 files changed, 52 insertions(+), 91 deletions(-) diff --git a/stratosphere/fs_mitm/source/fsmitm_worker.cpp b/stratosphere/fs_mitm/source/fsmitm_worker.cpp index 5146e8135..a3c755272 100644 --- a/stratosphere/fs_mitm/source/fsmitm_worker.cpp +++ b/stratosphere/fs_mitm/source/fsmitm_worker.cpp @@ -1,3 +1,4 @@ +#include #include #include #include "fsmitm_worker.hpp" @@ -18,10 +19,9 @@ Result FsMitMWorker::AddWaitableCallback(void *arg, Handle *handles, size_t num_ void FsMitMWorker::AddWaitable(IWaitable *waitable) { g_worker_waiter->add_waitable(waitable); - g_new_waitable_mutex.Lock(); + std::scoped_lock lk{g_new_waitable_mutex}; g_new_waitable_event->signal_event(); g_sema_new_waitable_finish.Wait(); - g_new_waitable_mutex.Unlock(); } void FsMitMWorker::Main(void *arg) { diff --git a/stratosphere/fs_mitm/source/mitm_query_service.cpp b/stratosphere/fs_mitm/source/mitm_query_service.cpp index db3d2be6e..41f78f905 100644 --- a/stratosphere/fs_mitm/source/mitm_query_service.cpp +++ b/stratosphere/fs_mitm/source/mitm_query_service.cpp @@ -1,3 +1,4 @@ +#include #include #include #include "mitm_query_service.hpp" @@ -8,7 +9,7 @@ static HosMutex g_pid_tid_mutex; Result MitMQueryUtils::get_associated_tid_for_pid(u64 pid, u64 *tid) { Result rc = 0xCAFE; - g_pid_tid_mutex.Lock(); + std::scoped_lock lk{g_pid_tid_mutex}; for (unsigned int i = 0; i < g_known_pids.size(); i++) { if (g_known_pids[i] == pid) { *tid = g_known_tids[i]; @@ -16,13 +17,11 @@ Result MitMQueryUtils::get_associated_tid_for_pid(u64 pid, u64 *tid) { break; } } - g_pid_tid_mutex.Unlock(); return rc; } void MitMQueryUtils::associate_pid_to_tid(u64 pid, u64 tid) { - g_pid_tid_mutex.Lock(); + std::scoped_lock lk{g_pid_tid_mutex}; g_known_pids.push_back(pid); g_known_tids.push_back(tid); - g_pid_tid_mutex.Unlock(); -} \ No newline at end of file +} diff --git a/stratosphere/libstratosphere/include/stratosphere/hossynch.hpp b/stratosphere/libstratosphere/include/stratosphere/hossynch.hpp index 6a1c6e32d..d75379aab 100644 --- a/stratosphere/libstratosphere/include/stratosphere/hossynch.hpp +++ b/stratosphere/libstratosphere/include/stratosphere/hossynch.hpp @@ -9,15 +9,15 @@ class HosMutex { mutexInit(&this->m); } - void Lock() { + void lock() { mutexLock(&this->m); } - void Unlock() { + void unlock() { mutexUnlock(&this->m); } - bool TryLock() { + bool try_lock() { return mutexTryLock(&this->m); } }; @@ -30,15 +30,15 @@ class HosRecursiveMutex { rmutexInit(&this->m); } - void Lock() { + void lock() { rmutexLock(&this->m); } - void Unlock() { + void unlock() { rmutexUnlock(&this->m); } - bool TryLock() { + bool try_lock() { return rmutexTryLock(&this->m); } }; diff --git a/stratosphere/libstratosphere/source/multithreadedwaitablemanager.cpp b/stratosphere/libstratosphere/source/multithreadedwaitablemanager.cpp index dcf1a931c..4d1dbfaec 100644 --- a/stratosphere/libstratosphere/source/multithreadedwaitablemanager.cpp +++ b/stratosphere/libstratosphere/source/multithreadedwaitablemanager.cpp @@ -2,6 +2,7 @@ #include #include +#include #include @@ -20,11 +21,10 @@ void MultiThreadedWaitableManager::process_until_timeout() { } void MultiThreadedWaitableManager::add_waitable(IWaitable *waitable) { - this->lock.Lock(); + std::scoped_lock lk{this->lock}; this->to_add_waitables.push_back(waitable); waitable->set_manager(this); this->new_waitable_event->signal_event(); - this->lock.Unlock(); } @@ -33,7 +33,7 @@ IWaitable *MultiThreadedWaitableManager::get_waitable() { int handle_index = 0; Result rc; - this->get_waitable_lock.Lock(); + std::scoped_lock lk{this->get_waitable_lock}; while (1) { /* Sort waitables by priority. */ std::sort(this->waitables.begin(), this->waitables.end(), IWaitable::compare); @@ -71,7 +71,6 @@ IWaitable *MultiThreadedWaitableManager::get_waitable() { w->handle_signaled(0); this->waitables.push_back(w); } else { - this->get_waitable_lock.Unlock(); return w; } } @@ -81,10 +80,9 @@ IWaitable *MultiThreadedWaitableManager::get_waitable() { Result MultiThreadedWaitableManager::add_waitable_callback(void *arg, Handle *handles, size_t num_handles, u64 timeout) { MultiThreadedWaitableManager *this_ptr = (MultiThreadedWaitableManager *)arg; svcClearEvent(handles[0]); - this_ptr->lock.Lock(); + std::scoped_lock lk{this_ptr->lock}; this_ptr->waitables.insert(this_ptr->waitables.end(), this_ptr->to_add_waitables.begin(), this_ptr->to_add_waitables.end()); this_ptr->to_add_waitables.clear(); - this_ptr->lock.Unlock(); return 0; } diff --git a/stratosphere/libstratosphere/source/waitablemanager.cpp b/stratosphere/libstratosphere/source/waitablemanager.cpp index 89b899629..615bf861d 100644 --- a/stratosphere/libstratosphere/source/waitablemanager.cpp +++ b/stratosphere/libstratosphere/source/waitablemanager.cpp @@ -2,15 +2,15 @@ #include #include +#include #include void WaitableManager::add_waitable(IWaitable *waitable) { - this->lock.Lock(); + std::scoped_lock lk{this->lock}; this->to_add_waitables.push_back(waitable); waitable->set_manager(this); this->has_new_items = true; - this->lock.Unlock(); } void WaitableManager::process_internal(bool break_on_timeout) { @@ -22,11 +22,10 @@ void WaitableManager::process_internal(bool break_on_timeout) { while (1) { /* Add new items, if relevant. */ if (this->has_new_items) { - this->lock.Lock(); + std::scoped_lock lk{this->lock}; this->waitables.insert(this->waitables.end(), this->to_add_waitables.begin(), this->to_add_waitables.end()); this->to_add_waitables.clear(); this->has_new_items = false; - this->lock.Unlock(); } /* Sort waitables by priority. */ diff --git a/stratosphere/pm/source/pm_debug_monitor.cpp b/stratosphere/pm/source/pm_debug_monitor.cpp index 6ac95f5da..cd85e1f8a 100644 --- a/stratosphere/pm/source/pm_debug_monitor.cpp +++ b/stratosphere/pm/source/pm_debug_monitor.cpp @@ -93,7 +93,7 @@ std::tuple DebugMonitorService::launch_debug_process(u64 pid) { } std::tuple DebugMonitorService::get_title_process_id(u64 tid) { - Registration::AutoProcessListLock auto_lock; + auto auto_lock = Registration::GetProcessListUniqueLock(); std::shared_ptr proc = Registration::GetProcessByTitleId(tid); if (proc != nullptr) { @@ -110,7 +110,7 @@ std::tuple DebugMonitorService::enable_debug_for_tid(u64 t } std::tuple DebugMonitorService::get_application_process_id() { - Registration::AutoProcessListLock auto_lock; + auto auto_lock = Registration::GetProcessListUniqueLock(); std::shared_ptr app_proc; if (Registration::HasApplicationProcess(&app_proc)) { diff --git a/stratosphere/pm/source/pm_info.cpp b/stratosphere/pm/source/pm_info.cpp index c4844f968..fa447fb97 100644 --- a/stratosphere/pm/source/pm_info.cpp +++ b/stratosphere/pm/source/pm_info.cpp @@ -22,7 +22,7 @@ Result InformationService::handle_deferred() { } std::tuple InformationService::get_title_id(u64 pid) { - Registration::AutoProcessListLock auto_lock; + auto auto_lock = Registration::GetProcessListUniqueLock(); std::shared_ptr proc = Registration::GetProcess(pid); if (proc != NULL) { diff --git a/stratosphere/pm/source/pm_process_wait.hpp b/stratosphere/pm/source/pm_process_wait.hpp index 7142f7b91..e074c03df 100644 --- a/stratosphere/pm/source/pm_process_wait.hpp +++ b/stratosphere/pm/source/pm_process_wait.hpp @@ -1,4 +1,5 @@ #pragma once +#include #include #include #include "pm_registration.hpp" @@ -36,18 +37,10 @@ class ProcessList final { public: std::vector> processes; - void Lock() { - this->mutex.Lock(); + auto get_unique_lock() { + return std::unique_lock{this->mutex}; } - - void Unlock() { - this->mutex.Unlock(); - } - - bool TryLock() { - return this->mutex.TryLock(); - } - + void set_manager(WaitableManager *manager) { this->manager = manager; } diff --git a/stratosphere/pm/source/pm_registration.cpp b/stratosphere/pm/source/pm_registration.cpp index d3c0c05b7..856e5dfec 100644 --- a/stratosphere/pm/source/pm_registration.cpp +++ b/stratosphere/pm/source/pm_registration.cpp @@ -22,25 +22,10 @@ static SystemEvent *g_process_event = NULL; static SystemEvent *g_debug_title_event = NULL; static SystemEvent *g_debug_application_event = NULL; -Registration::AutoProcessListLock::AutoProcessListLock() { - g_process_list.Lock(); - this->has_lock = true; +std::unique_lock Registration::GetProcessListUniqueLock() { + return g_process_list.get_unique_lock(); } -Registration::AutoProcessListLock::~AutoProcessListLock() { - if (this->has_lock) { - this->Unlock(); - } -} - -void Registration::AutoProcessListLock::Unlock() { - if (this->has_lock) { - g_process_list.Unlock(); - } - this->has_lock = false; -} - - void Registration::SetProcessListManager(WaitableManager *m) { g_process_list.set_manager(m); } @@ -185,7 +170,7 @@ HANDLE_PROCESS_LAUNCH_END: Result Registration::LaunchDebugProcess(u64 pid) { - AutoProcessListLock auto_lock; + auto auto_lock = GetProcessListUniqueLock(); LoaderProgramInfo program_info = {0}; Result rc; @@ -211,9 +196,8 @@ Result Registration::LaunchDebugProcess(u64 pid) { } Result Registration::LaunchProcess(u64 title_id, FsStorageId storage_id, u64 launch_flags, u64 *out_pid) { - Result rc; /* Only allow one mutex to exist. */ - g_process_launch_mutex.Lock(); + std::scoped_lock lk{g_process_launch_mutex}; g_process_launch_state.tid_sid.title_id = title_id; g_process_launch_state.tid_sid.storage_id = storage_id; g_process_launch_state.launch_flags = launch_flags; @@ -223,10 +207,7 @@ Result Registration::LaunchProcess(u64 title_id, FsStorageId storage_id, u64 lau g_process_launch_start_event->signal_event(); g_sema_finish_launch.Wait(); - rc = g_process_launch_state.result; - - g_process_launch_mutex.Unlock(); - return rc; + return g_process_launch_state.result; } Result Registration::LaunchProcessByTidSid(TidSid tid_sid, u64 launch_flags, u64 *out_pid) { @@ -292,7 +273,7 @@ Result Registration::HandleSignaledProcess(std::shared_ptr process) { - AutoProcessListLock auto_lock; + auto auto_lock = GetProcessListUniqueLock(); bool signal_debug_process_5x = kernelAbove500() && process->flags & 1; /* Unregister with FS. */ if (R_FAILED(fsprUnregisterProgram(process->pid))) { @@ -313,28 +294,27 @@ void Registration::FinalizeExitedProcess(std::shared_ptr /* Insert into dead process list, if relevant. */ if (signal_debug_process_5x) { - g_dead_process_list.Lock(); + auto lk = g_dead_process_list.get_unique_lock(); g_dead_process_list.processes.push_back(process); - g_dead_process_list.Unlock(); } /* Remove NOTE: This probably frees process. */ RemoveProcessFromList(process->pid); - auto_lock.Unlock(); + auto_lock.unlock(); if (signal_debug_process_5x) { g_process_event->signal_event(); } } void Registration::AddProcessToList(std::shared_ptr process) { - AutoProcessListLock auto_lock; + auto auto_lock = GetProcessListUniqueLock(); g_process_list.processes.push_back(process); g_process_list.get_manager()->add_waitable(new ProcessWaiter(process)); } void Registration::RemoveProcessFromList(u64 pid) { - AutoProcessListLock auto_lock; + auto auto_lock = GetProcessListUniqueLock(); /* Remove process from list. */ for (unsigned int i = 0; i < g_process_list.processes.size(); i++) { @@ -349,7 +329,7 @@ void Registration::RemoveProcessFromList(u64 pid) { } void Registration::SetProcessState(u64 pid, ProcessState new_state) { - AutoProcessListLock auto_lock; + auto auto_lock = GetProcessListUniqueLock(); /* Set process state. */ for (auto &process : g_process_list.processes) { @@ -361,7 +341,7 @@ void Registration::SetProcessState(u64 pid, ProcessState new_state) { } bool Registration::HasApplicationProcess(std::shared_ptr *out) { - AutoProcessListLock auto_lock; + auto auto_lock = GetProcessListUniqueLock(); for (auto &process : g_process_list.processes) { if (process->flags & 0x40) { @@ -376,7 +356,7 @@ bool Registration::HasApplicationProcess(std::shared_ptr } std::shared_ptr Registration::GetProcess(u64 pid) { - AutoProcessListLock auto_lock; + auto auto_lock = GetProcessListUniqueLock(); for (auto &process : g_process_list.processes) { if (process->pid == pid) { @@ -388,7 +368,7 @@ std::shared_ptr Registration::GetProcess(u64 pid) { } std::shared_ptr Registration::GetProcessByTitleId(u64 tid) { - AutoProcessListLock auto_lock; + auto auto_lock = GetProcessListUniqueLock(); for (auto &process : g_process_list.processes) { if (process->tid_sid.title_id == tid) { @@ -401,7 +381,7 @@ std::shared_ptr Registration::GetProcessByTitleId(u64 tid Result Registration::GetDebugProcessIds(u64 *out_pids, u32 max_out, u32 *num_out) { - AutoProcessListLock auto_lock; + auto auto_lock = GetProcessListUniqueLock(); u32 num = 0; @@ -420,7 +400,7 @@ Handle Registration::GetProcessEventHandle() { } void Registration::GetProcessEventType(u64 *out_pid, u64 *out_type) { - AutoProcessListLock auto_lock; + auto auto_lock = GetProcessListUniqueLock(); for (auto &p : g_process_list.processes) { if (kernelAbove200() && p->state >= ProcessState_DebugDetached && p->flags & 0x100) { @@ -448,17 +428,15 @@ void Registration::GetProcessEventType(u64 *out_pid, u64 *out_type) { } } if (kernelAbove500()) { - auto_lock.Unlock(); - g_dead_process_list.Lock(); + auto_lock.unlock(); + auto dead_process_list_lock = g_dead_process_list.get_unique_lock(); if (g_dead_process_list.processes.size()) { std::shared_ptr process = g_dead_process_list.processes[0]; g_dead_process_list.processes.erase(g_dead_process_list.processes.begin()); *out_pid = process->pid; *out_type = 1; - g_dead_process_list.Unlock(); return; } - g_dead_process_list.Unlock(); } *out_pid = 0; *out_type = 0; diff --git a/stratosphere/pm/source/pm_registration.hpp b/stratosphere/pm/source/pm_registration.hpp index 5e13b4143..ce22b8573 100644 --- a/stratosphere/pm/source/pm_registration.hpp +++ b/stratosphere/pm/source/pm_registration.hpp @@ -2,6 +2,7 @@ #include #include #include +#include #define LAUNCHFLAGS_NOTIFYWHENEXITED(flags) (flags & 1) #define LAUNCHFLAGS_STARTSUSPENDED(flags) (flags & (kernelAbove500() ? 0x10 : 0x2)) @@ -32,17 +33,10 @@ class Registration { u64* out_pid; Result result; }; - class AutoProcessListLock { - private: - bool has_lock; - public: - AutoProcessListLock(); - ~AutoProcessListLock(); - void Unlock(); - }; - + static void InitializeSystemResources(); static IWaitable *GetProcessLaunchStartEvent(); + static std::unique_lock GetProcessListUniqueLock(); static void SetProcessListManager(WaitableManager *m); static Result ProcessLaunchStartCallback(void *arg, Handle *handles, size_t num_handles, u64 timeout); diff --git a/stratosphere/pm/source/pm_shell.cpp b/stratosphere/pm/source/pm_shell.cpp index 282adb593..d09807eb8 100644 --- a/stratosphere/pm/source/pm_shell.cpp +++ b/stratosphere/pm/source/pm_shell.cpp @@ -90,7 +90,7 @@ std::tuple ShellService::launch_process(u64 launch_flags, Registrat } std::tuple ShellService::terminate_process_id(u64 pid) { - Registration::AutoProcessListLock auto_lock; + auto auto_lock = Registration::GetProcessListUniqueLock(); std::shared_ptr proc = Registration::GetProcess(pid); if (proc != NULL) { @@ -101,7 +101,7 @@ std::tuple ShellService::terminate_process_id(u64 pid) { } std::tuple ShellService::terminate_title_id(u64 tid) { - Registration::AutoProcessListLock auto_lock; + auto auto_lock = Registration::GetProcessListUniqueLock(); std::shared_ptr proc = Registration::GetProcessByTitleId(tid); if (proc != NULL) { @@ -122,7 +122,7 @@ std::tuple ShellService::get_process_event_type() { } std::tuple ShellService::finalize_exited_process(u64 pid) { - Registration::AutoProcessListLock auto_lock; + auto auto_lock = Registration::GetProcessListUniqueLock(); std::shared_ptr proc = Registration::GetProcess(pid); if (proc == NULL) { @@ -136,7 +136,7 @@ std::tuple ShellService::finalize_exited_process(u64 pid) { } std::tuple ShellService::clear_process_notification_flag(u64 pid) { - Registration::AutoProcessListLock auto_lock; + auto auto_lock = Registration::GetProcessListUniqueLock(); std::shared_ptr proc = Registration::GetProcess(pid); if (proc != NULL) { @@ -157,7 +157,7 @@ std::tuple ShellService::notify_boot_finished() { } std::tuple ShellService::get_application_process_id() { - Registration::AutoProcessListLock auto_lock; + auto auto_lock = Registration::GetProcessListUniqueLock(); std::shared_ptr app_proc; if (Registration::HasApplicationProcess(&app_proc)) {