From 5dfa7b74b5bef146705cc077e11293c5ac8d6fcb Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 19 Jul 2018 21:39:05 -0400 Subject: [PATCH 1/3] thread: Convert ThreadStatus into an enum class Makes the thread status strongly typed, so implicit conversions can't happen. It also makes it easier to catch mistakes at compile time. --- src/citra_qt/debugger/wait_tree.cpp | 46 ++++++++++----------- src/core/hle/kernel/address_arbiter.cpp | 7 ++-- src/core/hle/kernel/hle_ipc.cpp | 4 +- src/core/hle/kernel/server_session.cpp | 4 +- src/core/hle/kernel/svc.cpp | 20 ++++----- src/core/hle/kernel/thread.cpp | 54 ++++++++++++------------- src/core/hle/kernel/thread.h | 28 ++++++------- src/core/hle/kernel/wait_object.cpp | 12 +++--- src/core/hle/service/service.cpp | 4 +- 9 files changed, 89 insertions(+), 90 deletions(-) diff --git a/src/citra_qt/debugger/wait_tree.cpp b/src/citra_qt/debugger/wait_tree.cpp index e74db4467..8a6632978 100644 --- a/src/citra_qt/debugger/wait_tree.cpp +++ b/src/citra_qt/debugger/wait_tree.cpp @@ -145,32 +145,32 @@ QString WaitTreeThread::GetText() const { const auto& thread = static_cast(object); QString status; switch (thread.status) { - case THREADSTATUS_RUNNING: + case ThreadStatus::Running: status = tr("running"); break; - case THREADSTATUS_READY: + case ThreadStatus::Ready: status = tr("ready"); break; - case THREADSTATUS_WAIT_ARB: + case ThreadStatus::WaitArb: status = tr("waiting for address 0x%1").arg(thread.wait_address, 8, 16, QLatin1Char('0')); break; - case THREADSTATUS_WAIT_SLEEP: + case ThreadStatus::WaitSleep: status = tr("sleeping"); break; - case THREADSTATUS_WAIT_IPC: + case ThreadStatus::WaitIPC: status = tr("waiting for IPC response"); break; - case THREADSTATUS_WAIT_SYNCH_ALL: - case THREADSTATUS_WAIT_SYNCH_ANY: + case ThreadStatus::WaitSynchAll: + case ThreadStatus::WaitSynchAny: status = tr("waiting for objects"); break; - case THREADSTATUS_WAIT_HLE_EVENT: + case ThreadStatus::WaitHleEvent: status = tr("waiting for HLE return"); break; - case THREADSTATUS_DORMANT: + case ThreadStatus::Dormant: status = tr("dormant"); break; - case THREADSTATUS_DEAD: + case ThreadStatus::Dead: status = tr("dead"); break; } @@ -183,23 +183,23 @@ QString WaitTreeThread::GetText() const { QColor WaitTreeThread::GetColor() const { const auto& thread = static_cast(object); switch (thread.status) { - case THREADSTATUS_RUNNING: + case ThreadStatus::Running: return QColor(Qt::GlobalColor::darkGreen); - case THREADSTATUS_READY: + case ThreadStatus::Ready: return QColor(Qt::GlobalColor::darkBlue); - case THREADSTATUS_WAIT_ARB: + case ThreadStatus::WaitArb: return QColor(Qt::GlobalColor::darkRed); - case THREADSTATUS_WAIT_SLEEP: + case ThreadStatus::WaitSleep: return QColor(Qt::GlobalColor::darkYellow); - case THREADSTATUS_WAIT_IPC: + case ThreadStatus::WaitIPC: return QColor(Qt::GlobalColor::darkCyan); - case THREADSTATUS_WAIT_SYNCH_ALL: - case THREADSTATUS_WAIT_SYNCH_ANY: - case THREADSTATUS_WAIT_HLE_EVENT: + case ThreadStatus::WaitSynchAll: + case ThreadStatus::WaitSynchAny: + case ThreadStatus::WaitHleEvent: return QColor(Qt::GlobalColor::red); - case THREADSTATUS_DORMANT: + case ThreadStatus::Dormant: return QColor(Qt::GlobalColor::darkCyan); - case THREADSTATUS_DEAD: + case ThreadStatus::Dead: return QColor(Qt::GlobalColor::gray); default: return WaitTreeItem::GetColor(); @@ -243,9 +243,9 @@ std::vector> WaitTreeThread::GetChildren() const { } else { list.push_back(std::make_unique(thread.held_mutexes)); } - if (thread.status == THREADSTATUS_WAIT_SYNCH_ANY || - thread.status == THREADSTATUS_WAIT_SYNCH_ALL || - thread.status == THREADSTATUS_WAIT_HLE_EVENT) { + if (thread.status == ThreadStatus::WaitSynchAny || + thread.status == ThreadStatus::WaitSynchAll || + thread.status == ThreadStatus::WaitHleEvent) { list.push_back(std::make_unique(thread.wait_objects, thread.IsSleepingOnWaitAll())); } diff --git a/src/core/hle/kernel/address_arbiter.cpp b/src/core/hle/kernel/address_arbiter.cpp index 7baad55b1..d7894ee07 100644 --- a/src/core/hle/kernel/address_arbiter.cpp +++ b/src/core/hle/kernel/address_arbiter.cpp @@ -17,7 +17,7 @@ namespace Kernel { void AddressArbiter::WaitThread(SharedPtr thread, VAddr wait_address) { thread->wait_address = wait_address; - thread->status = THREADSTATUS_WAIT_ARB; + thread->status = ThreadStatus::WaitArb; waiting_threads.emplace_back(std::move(thread)); } @@ -25,7 +25,7 @@ void AddressArbiter::ResumeAllThreads(VAddr address) { // Determine which threads are waiting on this address, those should be woken up. auto itr = std::stable_partition(waiting_threads.begin(), waiting_threads.end(), [address](const auto& thread) { - ASSERT_MSG(thread->status == THREADSTATUS_WAIT_ARB, + ASSERT_MSG(thread->status == ThreadStatus::WaitArb, "Inconsistent AddressArbiter state"); return thread->wait_address != address; }); @@ -41,8 +41,7 @@ SharedPtr AddressArbiter::ResumeHighestPriorityThread(VAddr address) { // Determine which threads are waiting on this address, those should be considered for wakeup. auto matches_start = std::stable_partition( waiting_threads.begin(), waiting_threads.end(), [address](const auto& thread) { - ASSERT_MSG(thread->status == THREADSTATUS_WAIT_ARB, - "Inconsistent AddressArbiter state"); + ASSERT_MSG(thread->status == ThreadStatus::WaitArb, "Inconsistent AddressArbiter state"); return thread->wait_address != address; }); diff --git a/src/core/hle/kernel/hle_ipc.cpp b/src/core/hle/kernel/hle_ipc.cpp index 7c5f53452..cb63fe5e3 100644 --- a/src/core/hle/kernel/hle_ipc.cpp +++ b/src/core/hle/kernel/hle_ipc.cpp @@ -39,7 +39,7 @@ SharedPtr HLERequestContext::SleepClientThread(SharedPtr thread, thread->wakeup_callback = [context = *this, callback](ThreadWakeupReason reason, SharedPtr thread, SharedPtr object) mutable { - ASSERT(thread->status == THREADSTATUS_WAIT_HLE_EVENT); + ASSERT(thread->status == ThreadStatus::WaitHleEvent); callback(thread, context, reason); auto& process = thread->owner_process; @@ -56,7 +56,7 @@ SharedPtr HLERequestContext::SleepClientThread(SharedPtr thread, }; auto event = Kernel::Event::Create(Kernel::ResetType::OneShot, "HLE Pause Event: " + reason); - thread->status = THREADSTATUS_WAIT_HLE_EVENT; + thread->status = ThreadStatus::WaitHleEvent; thread->wait_objects = {event}; event->AddWaitingThread(thread); diff --git a/src/core/hle/kernel/server_session.cpp b/src/core/hle/kernel/server_session.cpp index b67de4540..71f702b9e 100644 --- a/src/core/hle/kernel/server_session.cpp +++ b/src/core/hle/kernel/server_session.cpp @@ -69,10 +69,10 @@ ResultCode ServerSession::HandleSyncRequest(SharedPtr thread) { hle_handler->HandleSyncRequest(SharedPtr(this)); } - if (thread->status == THREADSTATUS_RUNNING) { + if (thread->status == ThreadStatus::Running) { // Put the thread to sleep until the server replies, it will be awoken in // svcReplyAndReceive for LLE servers. - thread->status = THREADSTATUS_WAIT_IPC; + thread->status = ThreadStatus::WaitIPC; if (hle_handler != nullptr) { // For HLE services, we put the request threads to sleep for a short duration to diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index 615b3f911..36c8105b1 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -161,8 +161,8 @@ static void ExitProcess() { continue; // TODO(Subv): When are the other running/ready threads terminated? - ASSERT_MSG(thread->status == THREADSTATUS_WAIT_SYNCH_ANY || - thread->status == THREADSTATUS_WAIT_SYNCH_ALL, + ASSERT_MSG(thread->status == ThreadStatus::WaitSynchAny || + thread->status == ThreadStatus::WaitSynchAll, "Exiting processes with non-waiting threads is currently unimplemented"); thread->Stop(); @@ -283,14 +283,14 @@ static ResultCode WaitSynchronization1(Handle handle, s64 nano_seconds) { thread->wait_objects = {object}; object->AddWaitingThread(thread); - thread->status = THREADSTATUS_WAIT_SYNCH_ANY; + thread->status = ThreadStatus::WaitSynchAny; // Create an event to wake the thread up after the specified nanosecond delay has passed thread->WakeAfterDelay(nano_seconds); thread->wakeup_callback = [](ThreadWakeupReason reason, SharedPtr thread, SharedPtr object) { - ASSERT(thread->status == THREADSTATUS_WAIT_SYNCH_ANY); + ASSERT(thread->status == ThreadStatus::WaitSynchAny); if (reason == ThreadWakeupReason::Timeout) { thread->SetWaitSynchronizationResult(RESULT_TIMEOUT); @@ -365,7 +365,7 @@ static ResultCode WaitSynchronizationN(s32* out, VAddr handles_address, s32 hand return RESULT_TIMEOUT; // Put the thread to sleep - thread->status = THREADSTATUS_WAIT_SYNCH_ALL; + thread->status = ThreadStatus::WaitSynchAll; // Add the thread to each of the objects' waiting threads. for (auto& object : objects) { @@ -379,7 +379,7 @@ static ResultCode WaitSynchronizationN(s32* out, VAddr handles_address, s32 hand thread->wakeup_callback = [](ThreadWakeupReason reason, SharedPtr thread, SharedPtr object) { - ASSERT(thread->status == THREADSTATUS_WAIT_SYNCH_ALL); + ASSERT(thread->status == ThreadStatus::WaitSynchAll); if (reason == ThreadWakeupReason::Timeout) { thread->SetWaitSynchronizationResult(RESULT_TIMEOUT); @@ -421,7 +421,7 @@ static ResultCode WaitSynchronizationN(s32* out, VAddr handles_address, s32 hand return RESULT_TIMEOUT; // Put the thread to sleep - thread->status = THREADSTATUS_WAIT_SYNCH_ANY; + thread->status = ThreadStatus::WaitSynchAny; // Add the thread to each of the objects' waiting threads. for (std::size_t i = 0; i < objects.size(); ++i) { @@ -439,7 +439,7 @@ static ResultCode WaitSynchronizationN(s32* out, VAddr handles_address, s32 hand thread->wakeup_callback = [](ThreadWakeupReason reason, SharedPtr thread, SharedPtr object) { - ASSERT(thread->status == THREADSTATUS_WAIT_SYNCH_ANY); + ASSERT(thread->status == ThreadStatus::WaitSynchAny); if (reason == ThreadWakeupReason::Timeout) { thread->SetWaitSynchronizationResult(RESULT_TIMEOUT); @@ -578,7 +578,7 @@ static ResultCode ReplyAndReceive(s32* index, VAddr handles_address, s32 handle_ // No objects were ready to be acquired, prepare to suspend the thread. // Put the thread to sleep - thread->status = THREADSTATUS_WAIT_SYNCH_ANY; + thread->status = ThreadStatus::WaitSynchAny; // Add the thread to each of the objects' waiting threads. for (std::size_t i = 0; i < objects.size(); ++i) { @@ -590,7 +590,7 @@ static ResultCode ReplyAndReceive(s32* index, VAddr handles_address, s32 handle_ thread->wakeup_callback = [](ThreadWakeupReason reason, SharedPtr thread, SharedPtr object) { - ASSERT(thread->status == THREADSTATUS_WAIT_SYNCH_ANY); + ASSERT(thread->status == ThreadStatus::WaitSynchAny); ASSERT(reason == ThreadWakeupReason::Signal); ResultCode result = RESULT_SUCCESS; diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index e9d1c811c..174aeaf89 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -30,7 +30,7 @@ namespace Kernel { static CoreTiming::EventType* ThreadWakeupEventType = nullptr; bool Thread::ShouldWait(Thread* thread) const { - return status != THREADSTATUS_DEAD; + return status != ThreadStatus::Dead; } void Thread::Acquire(Thread* thread) { @@ -75,11 +75,11 @@ void Thread::Stop() { // Clean up thread from ready queue // This is only needed when the thread is termintated forcefully (SVC TerminateProcess) - if (status == THREADSTATUS_READY) { + if (status == ThreadStatus::Ready) { ready_queue.remove(current_priority, this); } - status = THREADSTATUS_DEAD; + status = ThreadStatus::Dead; WakeupAllWaitingThreads(); @@ -111,17 +111,17 @@ static void SwitchContext(Thread* new_thread) { previous_thread->last_running_ticks = CoreTiming::GetTicks(); Core::CPU().SaveContext(previous_thread->context); - if (previous_thread->status == THREADSTATUS_RUNNING) { + if (previous_thread->status == ThreadStatus::Running) { // This is only the case when a reschedule is triggered without the current thread // yielding execution (i.e. an event triggered, system core time-sliced, etc) ready_queue.push_front(previous_thread->current_priority, previous_thread); - previous_thread->status = THREADSTATUS_READY; + previous_thread->status = ThreadStatus::Ready; } } // Load context of new thread if (new_thread) { - ASSERT_MSG(new_thread->status == THREADSTATUS_READY, + ASSERT_MSG(new_thread->status == ThreadStatus::Ready, "Thread must be ready to become running."); // Cancel any outstanding wakeup events for this thread @@ -132,7 +132,7 @@ static void SwitchContext(Thread* new_thread) { current_thread = new_thread; ready_queue.remove(new_thread->current_priority, new_thread); - new_thread->status = THREADSTATUS_RUNNING; + new_thread->status = ThreadStatus::Running; if (previous_process != current_thread->owner_process) { Kernel::g_current_process = current_thread->owner_process; @@ -156,7 +156,7 @@ static Thread* PopNextReadyThread() { Thread* next; Thread* thread = GetCurrentThread(); - if (thread && thread->status == THREADSTATUS_RUNNING) { + if (thread && thread->status == ThreadStatus::Running) { // We have to do better than the current thread. // This call returns null when that's not possible. next = ready_queue.pop_first_better(thread->current_priority); @@ -173,7 +173,7 @@ static Thread* PopNextReadyThread() { void WaitCurrentThread_Sleep() { Thread* thread = GetCurrentThread(); - thread->status = THREADSTATUS_WAIT_SLEEP; + thread->status = ThreadStatus::WaitSleep; } void ExitCurrentThread() { @@ -195,9 +195,9 @@ static void ThreadWakeupCallback(u64 thread_handle, s64 cycles_late) { return; } - if (thread->status == THREADSTATUS_WAIT_SYNCH_ANY || - thread->status == THREADSTATUS_WAIT_SYNCH_ALL || thread->status == THREADSTATUS_WAIT_ARB || - thread->status == THREADSTATUS_WAIT_HLE_EVENT) { + if (thread->status == ThreadStatus::WaitSynchAny || + thread->status == ThreadStatus::WaitSynchAll || thread->status == ThreadStatus::WaitArb || + thread->status == ThreadStatus::WaitHleEvent) { // Invoke the wakeup callback before clearing the wait objects if (thread->wakeup_callback) @@ -224,27 +224,27 @@ void Thread::ResumeFromWait() { ASSERT_MSG(wait_objects.empty(), "Thread is waking up while waiting for objects"); switch (status) { - case THREADSTATUS_WAIT_SYNCH_ALL: - case THREADSTATUS_WAIT_SYNCH_ANY: - case THREADSTATUS_WAIT_HLE_EVENT: - case THREADSTATUS_WAIT_ARB: - case THREADSTATUS_WAIT_SLEEP: - case THREADSTATUS_WAIT_IPC: + case ThreadStatus::WaitSynchAll: + case ThreadStatus::WaitSynchAny: + case ThreadStatus::WaitHleEvent: + case ThreadStatus::WaitArb: + case ThreadStatus::WaitSleep: + case ThreadStatus::WaitIPC: break; - case THREADSTATUS_READY: + case ThreadStatus::Ready: // The thread's wakeup callback must have already been cleared when the thread was first // awoken. ASSERT(wakeup_callback == nullptr); // If the thread is waiting on multiple wait objects, it might be awoken more than once // before actually resuming. We can ignore subsequent wakeups if the thread status has - // already been set to THREADSTATUS_READY. + // already been set to ThreadStatus::Ready. return; - case THREADSTATUS_RUNNING: + case ThreadStatus::Running: DEBUG_ASSERT_MSG(false, "Thread with object id {} has already resumed.", GetObjectId()); return; - case THREADSTATUS_DEAD: + case ThreadStatus::Dead: // This should never happen, as threads must complete before being stopped. DEBUG_ASSERT_MSG(false, "Thread with object id {} cannot be resumed because it's DEAD.", GetObjectId()); @@ -254,7 +254,7 @@ void Thread::ResumeFromWait() { wakeup_callback = nullptr; ready_queue.push_back(current_priority, this); - status = THREADSTATUS_READY; + status = ThreadStatus::Ready; Core::System::GetInstance().PrepareReschedule(); } @@ -349,7 +349,7 @@ ResultVal> Thread::Create(std::string name, VAddr entry_point, ready_queue.prepare(priority); thread->thread_id = NewThreadId(); - thread->status = THREADSTATUS_DORMANT; + thread->status = ThreadStatus::Dormant; thread->entry_point = entry_point; thread->stack_top = stack_top; thread->nominal_priority = thread->current_priority = priority; @@ -408,7 +408,7 @@ ResultVal> Thread::Create(std::string name, VAddr entry_point, ResetThreadContext(thread->context, stack_top, entry_point, arg); ready_queue.push_back(thread->current_priority, thread.get()); - thread->status = THREADSTATUS_READY; + thread->status = ThreadStatus::Ready; return MakeResult>(std::move(thread)); } @@ -417,7 +417,7 @@ void Thread::SetPriority(u32 priority) { ASSERT_MSG(priority <= THREADPRIO_LOWEST && priority >= THREADPRIO_HIGHEST, "Invalid priority value."); // If thread was ready, adjust queues - if (status == THREADSTATUS_READY) + if (status == ThreadStatus::Ready) ready_queue.move(this, current_priority, priority); else ready_queue.prepare(priority); @@ -436,7 +436,7 @@ void Thread::UpdatePriority() { void Thread::BoostPriority(u32 priority) { // If thread was ready, adjust queues - if (status == THREADSTATUS_READY) + if (status == ThreadStatus::Ready) ready_queue.move(this, current_priority, priority); else ready_queue.prepare(priority); diff --git a/src/core/hle/kernel/thread.h b/src/core/hle/kernel/thread.h index f1d80a1d5..cb5b29f6b 100644 --- a/src/core/hle/kernel/thread.h +++ b/src/core/hle/kernel/thread.h @@ -31,16 +31,16 @@ enum ThreadProcessorId : s32 { }; enum ThreadStatus { - THREADSTATUS_RUNNING, ///< Currently running - THREADSTATUS_READY, ///< Ready to run - THREADSTATUS_WAIT_ARB, ///< Waiting on an address arbiter - THREADSTATUS_WAIT_SLEEP, ///< Waiting due to a SleepThread SVC - THREADSTATUS_WAIT_IPC, ///< Waiting for the reply from an IPC request - THREADSTATUS_WAIT_SYNCH_ANY, ///< Waiting due to WaitSynch1 or WaitSynchN with wait_all = false - THREADSTATUS_WAIT_SYNCH_ALL, ///< Waiting due to WaitSynchronizationN with wait_all = true - THREADSTATUS_WAIT_HLE_EVENT, ///< Waiting due to an HLE handler pausing the thread - THREADSTATUS_DORMANT, ///< Created but not yet made ready - THREADSTATUS_DEAD ///< Run to completion, or forcefully terminated + Running, ///< Currently running + Ready, ///< Ready to run + WaitArb, ///< Waiting on an address arbiter + WaitSleep, ///< Waiting due to a SleepThread SVC + WaitIPC, ///< Waiting for the reply from an IPC request + WaitSynchAny, ///< Waiting due to WaitSynch1 or WaitSynchN with wait_all = false + WaitSynchAll, ///< Waiting due to WaitSynchronizationN with wait_all = true + WaitHleEvent, ///< Waiting due to an HLE handler pausing the thread + Dormant, ///< Created but not yet made ready + Dead ///< Run to completion, or forcefully terminated }; enum class ThreadWakeupReason { @@ -178,16 +178,16 @@ public: * with wait_all = true. */ bool IsSleepingOnWaitAll() const { - return status == THREADSTATUS_WAIT_SYNCH_ALL; + return status == ThreadStatus::WaitSynchAll; } std::unique_ptr context; u32 thread_id; - u32 status; - u32 entry_point; - u32 stack_top; + ThreadStatus status; + VAddr entry_point; + VAddr stack_top; u32 nominal_priority; ///< Nominal thread priority, as set by the emulated application u32 current_priority; ///< Current thread priority, can be temporarily changed diff --git a/src/core/hle/kernel/wait_object.cpp b/src/core/hle/kernel/wait_object.cpp index 320e16d07..844556910 100644 --- a/src/core/hle/kernel/wait_object.cpp +++ b/src/core/hle/kernel/wait_object.cpp @@ -38,9 +38,9 @@ SharedPtr WaitObject::GetHighestPriorityReadyThread() { for (const auto& thread : waiting_threads) { // The list of waiting threads must not contain threads that are not waiting to be awakened. - ASSERT_MSG(thread->status == THREADSTATUS_WAIT_SYNCH_ANY || - thread->status == THREADSTATUS_WAIT_SYNCH_ALL || - thread->status == THREADSTATUS_WAIT_HLE_EVENT, + ASSERT_MSG(thread->status == ThreadStatus::WaitSynchAny || + thread->status == ThreadStatus::WaitSynchAll || + thread->status == ThreadStatus::WaitHleEvent, "Inconsistent thread statuses in waiting_threads"); if (thread->current_priority >= candidate_priority) @@ -49,10 +49,10 @@ SharedPtr WaitObject::GetHighestPriorityReadyThread() { if (ShouldWait(thread.get())) continue; - // A thread is ready to run if it's either in THREADSTATUS_WAIT_SYNCH_ANY or - // in THREADSTATUS_WAIT_SYNCH_ALL and the rest of the objects it is waiting on are ready. + // A thread is ready to run if it's either in ThreadStatus::WaitSynchAny or + // in ThreadStatus::WaitSynchAll and the rest of the objects it is waiting on are ready. bool ready_to_run = true; - if (thread->status == THREADSTATUS_WAIT_SYNCH_ALL) { + if (thread->status == ThreadStatus::WaitSynchAll) { ready_to_run = std::none_of(thread->wait_objects.begin(), thread->wait_objects.end(), [&thread](const SharedPtr& object) { return object->ShouldWait(thread.get()); diff --git a/src/core/hle/service/service.cpp b/src/core/hle/service/service.cpp index caad3f4b2..d6763d710 100644 --- a/src/core/hle/service/service.cpp +++ b/src/core/hle/service/service.cpp @@ -198,11 +198,11 @@ void ServiceFrameworkBase::HandleSyncRequest(SharedPtr server_ses handler_invoker(this, info->handler_callback, context); auto thread = Kernel::GetCurrentThread(); - ASSERT(thread->status == THREADSTATUS_RUNNING || thread->status == THREADSTATUS_WAIT_HLE_EVENT); + ASSERT(thread->status == ThreadStatus::Running || thread->status == ThreadStatus::WaitHleEvent); // Only write the response immediately if the thread is still running. If the HLE handler put // the thread to sleep then the writing of the command buffer will be deferred to the wakeup // callback. - if (thread->status == THREADSTATUS_RUNNING) { + if (thread->status == ThreadStatus::Running) { context.WriteToOutgoingCommandBuffer(cmd_buf, *Kernel::g_current_process, Kernel::g_handle_table); } From ca3d9d659ea3ead0079929be63df55cbab6917ed Mon Sep 17 00:00:00 2001 From: Lioncash Date: Thu, 13 Sep 2018 15:57:45 -0400 Subject: [PATCH 2/3] kernel/thread: Include thread-related enums within the kernel namespace Previously, these were sitting outside of the Kernel namespace, which doesn't really make sense, given they're related to the Thread class which is within the Kernel namespace. --- src/citra_qt/debugger/wait_tree.cpp | 54 ++++++++++++++-------------- src/core/hle/kernel/thread.h | 10 +++--- src/core/hle/service/fs/archive.cpp | 2 +- src/core/hle/service/nwm/nwm_uds.cpp | 2 +- src/core/hle/service/service.cpp | 5 +-- src/core/hle/service/sm/srv.cpp | 3 +- 6 files changed, 39 insertions(+), 37 deletions(-) diff --git a/src/citra_qt/debugger/wait_tree.cpp b/src/citra_qt/debugger/wait_tree.cpp index 8a6632978..f86899019 100644 --- a/src/citra_qt/debugger/wait_tree.cpp +++ b/src/citra_qt/debugger/wait_tree.cpp @@ -145,32 +145,32 @@ QString WaitTreeThread::GetText() const { const auto& thread = static_cast(object); QString status; switch (thread.status) { - case ThreadStatus::Running: + case Kernel::ThreadStatus::Running: status = tr("running"); break; - case ThreadStatus::Ready: + case Kernel::ThreadStatus::Ready: status = tr("ready"); break; - case ThreadStatus::WaitArb: + case Kernel::ThreadStatus::WaitArb: status = tr("waiting for address 0x%1").arg(thread.wait_address, 8, 16, QLatin1Char('0')); break; - case ThreadStatus::WaitSleep: + case Kernel::ThreadStatus::WaitSleep: status = tr("sleeping"); break; - case ThreadStatus::WaitIPC: + case Kernel::ThreadStatus::WaitIPC: status = tr("waiting for IPC response"); break; - case ThreadStatus::WaitSynchAll: - case ThreadStatus::WaitSynchAny: + case Kernel::ThreadStatus::WaitSynchAll: + case Kernel::ThreadStatus::WaitSynchAny: status = tr("waiting for objects"); break; - case ThreadStatus::WaitHleEvent: + case Kernel::ThreadStatus::WaitHleEvent: status = tr("waiting for HLE return"); break; - case ThreadStatus::Dormant: + case Kernel::ThreadStatus::Dormant: status = tr("dormant"); break; - case ThreadStatus::Dead: + case Kernel::ThreadStatus::Dead: status = tr("dead"); break; } @@ -183,23 +183,23 @@ QString WaitTreeThread::GetText() const { QColor WaitTreeThread::GetColor() const { const auto& thread = static_cast(object); switch (thread.status) { - case ThreadStatus::Running: + case Kernel::ThreadStatus::Running: return QColor(Qt::GlobalColor::darkGreen); - case ThreadStatus::Ready: + case Kernel::ThreadStatus::Ready: return QColor(Qt::GlobalColor::darkBlue); - case ThreadStatus::WaitArb: + case Kernel::ThreadStatus::WaitArb: return QColor(Qt::GlobalColor::darkRed); - case ThreadStatus::WaitSleep: + case Kernel::ThreadStatus::WaitSleep: return QColor(Qt::GlobalColor::darkYellow); - case ThreadStatus::WaitIPC: + case Kernel::ThreadStatus::WaitIPC: return QColor(Qt::GlobalColor::darkCyan); - case ThreadStatus::WaitSynchAll: - case ThreadStatus::WaitSynchAny: - case ThreadStatus::WaitHleEvent: + case Kernel::ThreadStatus::WaitSynchAll: + case Kernel::ThreadStatus::WaitSynchAny: + case Kernel::ThreadStatus::WaitHleEvent: return QColor(Qt::GlobalColor::red); - case ThreadStatus::Dormant: + case Kernel::ThreadStatus::Dormant: return QColor(Qt::GlobalColor::darkCyan); - case ThreadStatus::Dead: + case Kernel::ThreadStatus::Dead: return QColor(Qt::GlobalColor::gray); default: return WaitTreeItem::GetColor(); @@ -213,16 +213,16 @@ std::vector> WaitTreeThread::GetChildren() const { QString processor; switch (thread.processor_id) { - case ThreadProcessorId::THREADPROCESSORID_DEFAULT: + case Kernel::ThreadProcessorId::THREADPROCESSORID_DEFAULT: processor = tr("default"); break; - case ThreadProcessorId::THREADPROCESSORID_ALL: + case Kernel::ThreadProcessorId::THREADPROCESSORID_ALL: processor = tr("all"); break; - case ThreadProcessorId::THREADPROCESSORID_0: + case Kernel::ThreadProcessorId::THREADPROCESSORID_0: processor = tr("AppCore"); break; - case ThreadProcessorId::THREADPROCESSORID_1: + case Kernel::ThreadProcessorId::THREADPROCESSORID_1: processor = tr("SysCore"); break; default: @@ -243,9 +243,9 @@ std::vector> WaitTreeThread::GetChildren() const { } else { list.push_back(std::make_unique(thread.held_mutexes)); } - if (thread.status == ThreadStatus::WaitSynchAny || - thread.status == ThreadStatus::WaitSynchAll || - thread.status == ThreadStatus::WaitHleEvent) { + if (thread.status == Kernel::ThreadStatus::WaitSynchAny || + thread.status == Kernel::ThreadStatus::WaitSynchAll || + thread.status == Kernel::ThreadStatus::WaitHleEvent) { list.push_back(std::make_unique(thread.wait_objects, thread.IsSleepingOnWaitAll())); } diff --git a/src/core/hle/kernel/thread.h b/src/core/hle/kernel/thread.h index cb5b29f6b..5e9b2d6b5 100644 --- a/src/core/hle/kernel/thread.h +++ b/src/core/hle/kernel/thread.h @@ -15,6 +15,11 @@ #include "core/hle/kernel/wait_object.h" #include "core/hle/result.h" +namespace Kernel { + +class Mutex; +class Process; + enum ThreadPriority : u32 { THREADPRIO_HIGHEST = 0, ///< Highest thread priority THREADPRIO_USERLAND_MAX = 24, ///< Highest thread priority for userland apps @@ -48,11 +53,6 @@ enum class ThreadWakeupReason { Timeout // The thread was woken up due to a wait timeout. }; -namespace Kernel { - -class Mutex; -class Process; - class Thread final : public WaitObject { public: /** diff --git a/src/core/hle/service/fs/archive.cpp b/src/core/hle/service/fs/archive.cpp index 79fb1847f..b653de29f 100644 --- a/src/core/hle/service/fs/archive.cpp +++ b/src/core/hle/service/fs/archive.cpp @@ -108,7 +108,7 @@ void File::Read(Kernel::HLERequestContext& ctx) { std::chrono::nanoseconds read_timeout_ns{backend->GetReadDelayNs(length)}; ctx.SleepClientThread(Kernel::GetCurrentThread(), "file::read", read_timeout_ns, [](Kernel::SharedPtr thread, - Kernel::HLERequestContext& ctx, ThreadWakeupReason reason) { + Kernel::HLERequestContext& ctx, Kernel::ThreadWakeupReason reason) { // Nothing to do here }); } diff --git a/src/core/hle/service/nwm/nwm_uds.cpp b/src/core/hle/service/nwm/nwm_uds.cpp index 8e0dfece9..e54f50763 100644 --- a/src/core/hle/service/nwm/nwm_uds.cpp +++ b/src/core/hle/service/nwm/nwm_uds.cpp @@ -1162,7 +1162,7 @@ void NWM_UDS::ConnectToNetwork(Kernel::HLERequestContext& ctx) { connection_event = ctx.SleepClientThread( Kernel::GetCurrentThread(), "uds::ConnectToNetwork", UDSConnectionTimeout, [](Kernel::SharedPtr thread, Kernel::HLERequestContext& ctx, - ThreadWakeupReason reason) { + Kernel::ThreadWakeupReason reason) { // TODO(B3N30): Add error handling for host full and timeout IPC::RequestBuilder rb(ctx, 0x1E, 1, 0); rb.Push(RESULT_SUCCESS); diff --git a/src/core/hle/service/service.cpp b/src/core/hle/service/service.cpp index d6763d710..d99d07aac 100644 --- a/src/core/hle/service/service.cpp +++ b/src/core/hle/service/service.cpp @@ -198,11 +198,12 @@ void ServiceFrameworkBase::HandleSyncRequest(SharedPtr server_ses handler_invoker(this, info->handler_callback, context); auto thread = Kernel::GetCurrentThread(); - ASSERT(thread->status == ThreadStatus::Running || thread->status == ThreadStatus::WaitHleEvent); + ASSERT(thread->status == Kernel::ThreadStatus::Running || + thread->status == Kernel::ThreadStatus::WaitHleEvent); // Only write the response immediately if the thread is still running. If the HLE handler put // the thread to sleep then the writing of the command buffer will be deferred to the wakeup // callback. - if (thread->status == ThreadStatus::Running) { + if (thread->status == Kernel::ThreadStatus::Running) { context.WriteToOutgoingCommandBuffer(cmd_buf, *Kernel::g_current_process, Kernel::g_handle_table); } diff --git a/src/core/hle/service/sm/srv.cpp b/src/core/hle/service/sm/srv.cpp index a8f2db4f0..6e96a88e0 100644 --- a/src/core/hle/service/sm/srv.cpp +++ b/src/core/hle/service/sm/srv.cpp @@ -101,7 +101,8 @@ void SRV::GetServiceHandle(Kernel::HLERequestContext& ctx) { // TODO(yuriks): Permission checks go here auto get_handle = [name, this](Kernel::SharedPtr thread, - Kernel::HLERequestContext& ctx, ThreadWakeupReason reason) { + Kernel::HLERequestContext& ctx, + Kernel::ThreadWakeupReason reason) { LOG_ERROR(Service_SRV, "called service={} wakeup", name); auto client_port = service_manager->GetServicePort(name); From 3ee9f669c136589217f57cbf5e77e74acac9206f Mon Sep 17 00:00:00 2001 From: fearlessTobi Date: Fri, 21 Sep 2018 16:39:10 +0200 Subject: [PATCH 3/3] Address review comments --- src/citra_qt/debugger/wait_tree.cpp | 8 ++++---- src/core/hle/kernel/address_arbiter.cpp | 3 ++- src/core/hle/kernel/mutex.cpp | 2 +- src/core/hle/kernel/svc.cpp | 14 +++++++------- src/core/hle/kernel/thread.cpp | 8 ++++---- src/core/hle/kernel/thread.h | 20 ++++++++++---------- src/core/hle/kernel/wait_object.cpp | 2 +- 7 files changed, 29 insertions(+), 28 deletions(-) diff --git a/src/citra_qt/debugger/wait_tree.cpp b/src/citra_qt/debugger/wait_tree.cpp index f86899019..d006128e5 100644 --- a/src/citra_qt/debugger/wait_tree.cpp +++ b/src/citra_qt/debugger/wait_tree.cpp @@ -213,16 +213,16 @@ std::vector> WaitTreeThread::GetChildren() const { QString processor; switch (thread.processor_id) { - case Kernel::ThreadProcessorId::THREADPROCESSORID_DEFAULT: + case Kernel::ThreadProcessorId::ThreadProcessorIdDefault: processor = tr("default"); break; - case Kernel::ThreadProcessorId::THREADPROCESSORID_ALL: + case Kernel::ThreadProcessorId::ThreadProcessorIdAll: processor = tr("all"); break; - case Kernel::ThreadProcessorId::THREADPROCESSORID_0: + case Kernel::ThreadProcessorId::ThreadProcessorId0: processor = tr("AppCore"); break; - case Kernel::ThreadProcessorId::THREADPROCESSORID_1: + case Kernel::ThreadProcessorId::ThreadProcessorId1: processor = tr("SysCore"); break; default: diff --git a/src/core/hle/kernel/address_arbiter.cpp b/src/core/hle/kernel/address_arbiter.cpp index d7894ee07..1dc005554 100644 --- a/src/core/hle/kernel/address_arbiter.cpp +++ b/src/core/hle/kernel/address_arbiter.cpp @@ -41,7 +41,8 @@ SharedPtr AddressArbiter::ResumeHighestPriorityThread(VAddr address) { // Determine which threads are waiting on this address, those should be considered for wakeup. auto matches_start = std::stable_partition( waiting_threads.begin(), waiting_threads.end(), [address](const auto& thread) { - ASSERT_MSG(thread->status == ThreadStatus::WaitArb, "Inconsistent AddressArbiter state"); + ASSERT_MSG(thread->status == ThreadStatus::WaitArb, + "Inconsistent AddressArbiter state"); return thread->wait_address != address; }); diff --git a/src/core/hle/kernel/mutex.cpp b/src/core/hle/kernel/mutex.cpp index 3593ab2f8..02cbf613c 100644 --- a/src/core/hle/kernel/mutex.cpp +++ b/src/core/hle/kernel/mutex.cpp @@ -109,7 +109,7 @@ void Mutex::UpdatePriority() { if (!holding_thread) return; - u32 best_priority = THREADPRIO_LOWEST; + u32 best_priority = ThreadPrioLowest; for (auto& waiter : GetWaitingThreads()) { if (waiter->current_priority < best_priority) best_priority = waiter->current_priority; diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index 36c8105b1..7ee0bbfa9 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -725,7 +725,7 @@ static ResultCode CreateThread(Handle* out_handle, u32 priority, u32 entry_point u32 stack_top, s32 processor_id) { std::string name = fmt::format("thread-{:08X}", entry_point); - if (priority > THREADPRIO_LOWEST) { + if (priority > ThreadPrioLowest) { return ERR_OUT_OF_RANGE; } @@ -734,20 +734,20 @@ static ResultCode CreateThread(Handle* out_handle, u32 priority, u32 entry_point return ERR_NOT_AUTHORIZED; } - if (processor_id == THREADPROCESSORID_DEFAULT) { + if (processor_id == ThreadProcessorIdDefault) { // Set the target CPU to the one specified in the process' exheader. processor_id = g_current_process->ideal_processor; - ASSERT(processor_id != THREADPROCESSORID_DEFAULT); + ASSERT(processor_id != ThreadProcessorIdDefault); } switch (processor_id) { - case THREADPROCESSORID_0: + case ThreadProcessorId0: break; - case THREADPROCESSORID_ALL: + case ThreadProcessorIdAll: LOG_INFO(Kernel_SVC, "Newly created thread is allowed to be run in any Core, unimplemented."); break; - case THREADPROCESSORID_1: + case ThreadProcessorId1: LOG_ERROR(Kernel_SVC, "Newly created thread must run in the SysCore (Core1), unimplemented."); break; @@ -796,7 +796,7 @@ static ResultCode GetThreadPriority(u32* priority, Handle handle) { /// Sets the priority for the specified thread static ResultCode SetThreadPriority(Handle handle, u32 priority) { - if (priority > THREADPRIO_LOWEST) { + if (priority > ThreadPrioLowest) { return ERR_OUT_OF_RANGE; } diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index 174aeaf89..50fa061c4 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -45,7 +45,7 @@ static Kernel::HandleTable wakeup_callback_handle_table; static std::vector> thread_list; // Lists only ready thread ids. -static Common::ThreadQueueList ready_queue; +static Common::ThreadQueueList ready_queue; static SharedPtr current_thread; @@ -324,12 +324,12 @@ ResultVal> Thread::Create(std::string name, VAddr entry_point, u32 arg, s32 processor_id, VAddr stack_top, SharedPtr owner_process) { // Check if priority is in ranged. Lowest priority -> highest priority id. - if (priority > THREADPRIO_LOWEST) { + if (priority > ThreadPrioLowest) { LOG_ERROR(Kernel_SVC, "Invalid thread priority: {}", priority); return ERR_OUT_OF_RANGE; } - if (processor_id > THREADPROCESSORID_MAX) { + if (processor_id > ThreadProcessorIdMax) { LOG_ERROR(Kernel_SVC, "Invalid processor id: {}", processor_id); return ERR_OUT_OF_RANGE_KERNEL; } @@ -414,7 +414,7 @@ ResultVal> Thread::Create(std::string name, VAddr entry_point, } void Thread::SetPriority(u32 priority) { - ASSERT_MSG(priority <= THREADPRIO_LOWEST && priority >= THREADPRIO_HIGHEST, + ASSERT_MSG(priority <= ThreadPrioLowest && priority >= ThreadPrioHighest, "Invalid priority value."); // If thread was ready, adjust queues if (status == ThreadStatus::Ready) diff --git a/src/core/hle/kernel/thread.h b/src/core/hle/kernel/thread.h index 5e9b2d6b5..3fd72563f 100644 --- a/src/core/hle/kernel/thread.h +++ b/src/core/hle/kernel/thread.h @@ -21,21 +21,21 @@ class Mutex; class Process; enum ThreadPriority : u32 { - THREADPRIO_HIGHEST = 0, ///< Highest thread priority - THREADPRIO_USERLAND_MAX = 24, ///< Highest thread priority for userland apps - THREADPRIO_DEFAULT = 48, ///< Default thread priority for userland apps - THREADPRIO_LOWEST = 63, ///< Lowest thread priority + ThreadPrioHighest = 0, ///< Highest thread priority + ThreadPrioUserlandMax = 24, ///< Highest thread priority for userland apps + ThreadPrioDefault = 48, ///< Default thread priority for userland apps + ThreadPrioLowest = 63, ///< Lowest thread priority }; enum ThreadProcessorId : s32 { - THREADPROCESSORID_DEFAULT = -2, ///< Run thread on default core specified by exheader - THREADPROCESSORID_ALL = -1, ///< Run thread on either core - THREADPROCESSORID_0 = 0, ///< Run thread on core 0 (AppCore) - THREADPROCESSORID_1 = 1, ///< Run thread on core 1 (SysCore) - THREADPROCESSORID_MAX = 2, ///< Processor ID must be less than this + ThreadProcessorIdDefault = -2, ///< Run thread on default core specified by exheader + ThreadProcessorIdAll = -1, ///< Run thread on either core + ThreadProcessorId0 = 0, ///< Run thread on core 0 (AppCore) + ThreadProcessorId1 = 1, ///< Run thread on core 1 (SysCore) + ThreadProcessorIdMax = 2, ///< Processor ID must be less than this }; -enum ThreadStatus { +enum class ThreadStatus { Running, ///< Currently running Ready, ///< Ready to run WaitArb, ///< Waiting on an address arbiter diff --git a/src/core/hle/kernel/wait_object.cpp b/src/core/hle/kernel/wait_object.cpp index 844556910..12699f85e 100644 --- a/src/core/hle/kernel/wait_object.cpp +++ b/src/core/hle/kernel/wait_object.cpp @@ -34,7 +34,7 @@ void WaitObject::RemoveWaitingThread(Thread* thread) { SharedPtr WaitObject::GetHighestPriorityReadyThread() { Thread* candidate = nullptr; - u32 candidate_priority = THREADPRIO_LOWEST + 1; + u32 candidate_priority = ThreadPrioLowest + 1; for (const auto& thread : waiting_threads) { // The list of waiting threads must not contain threads that are not waiting to be awakened.