From e000aaaf5d904abe6691751d7319d2aca90ff922 Mon Sep 17 00:00:00 2001 From: comex Date: Mon, 13 Apr 2015 01:49:24 -0400 Subject: [PATCH 1/2] Have the UI thread do PauseAndLock before messing with GetUsbPointer. Since its lifetime is managed on the CPU thread, this (or a refactoring) is absolutely required. One of the functions with a PauseAndLock call added is CFrame::UpdateGUI; this is fine now, since it's called only after important events happen, so just make sure not to call it every frame or something :) --- Source/Core/Core/CoreTiming.cpp | 7 +++---- Source/Core/DolphinWX/FrameTools.cpp | 6 ++++++ 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/Source/Core/Core/CoreTiming.cpp b/Source/Core/Core/CoreTiming.cpp index d9872dcc3b..041c3b150e 100644 --- a/Source/Core/Core/CoreTiming.cpp +++ b/Source/Core/Core/CoreTiming.cpp @@ -224,8 +224,7 @@ u64 GetIdleTicks() // schedule things to be executed on the main thread. void ScheduleEvent_Threadsafe(int cyclesIntoFuture, int event_type, u64 userdata) { - // TODO: Fix UI thread safety problems, and enable this assertion - // _assert_msg_(POWERPC, !Core::IsCPUThread(), "ScheduleEvent_Threadsafe from wrong thread"); + _assert_msg_(POWERPC, !Core::IsCPUThread(), "ScheduleEvent_Threadsafe from wrong thread"); std::lock_guard lk(tsWriteLock); Event ne; ne.time = globalTimer + cyclesIntoFuture; @@ -287,8 +286,8 @@ static void AddEventToQueue(Event* ne) // than Advance void ScheduleEvent(int cyclesIntoFuture, int event_type, u64 userdata) { - // TODO: Fix UI thread safety problems, and enable this assertion - //_assert_msg_(POWERPC, Core::IsCPUThread(), "ScheduleEvent from wrong thread"); + _assert_msg_(POWERPC, Core::IsCPUThread() || Core::GetState() == Core::CORE_PAUSE, + "ScheduleEvent from wrong thread"); Event *ne = GetNewEvent(); ne->userdata = userdata; ne->type = event_type; diff --git a/Source/Core/DolphinWX/FrameTools.cpp b/Source/Core/DolphinWX/FrameTools.cpp index de41814259..4edb08980f 100644 --- a/Source/Core/DolphinWX/FrameTools.cpp +++ b/Source/Core/DolphinWX/FrameTools.cpp @@ -1584,17 +1584,21 @@ void CFrame::ConnectWiimote(int wm_idx, bool connect) { if (Core::IsRunning() && SConfig::GetInstance().m_LocalCoreStartupParameter.bWii) { + bool was_unpaused = Core::PauseAndLock(true); GetUsbPointer()->AccessWiiMote(wm_idx | 0x100)->Activate(connect); wxString msg(wxString::Format(_("Wiimote %i %s"), wm_idx + 1, connect ? _("Connected") : _("Disconnected"))); Core::DisplayMessage(WxStrToStr(msg), 3000); Host_UpdateMainFrame(); + Core::PauseAndLock(false, was_unpaused); } } void CFrame::OnConnectWiimote(wxCommandEvent& event) { + bool was_unpaused = Core::PauseAndLock(true); ConnectWiimote(event.GetId() - IDM_CONNECT_WIIMOTE1, !GetUsbPointer()->AccessWiiMote((event.GetId() - IDM_CONNECT_WIIMOTE1) | 0x100)->IsConnected()); + Core::PauseAndLock(false, was_unpaused); } // Toggle fullscreen. In Windows the fullscreen mode is accomplished by expanding the m_Panel to cover @@ -1792,6 +1796,7 @@ void CFrame::UpdateGUI() GetMenuBar()->FindItem(IDM_CONNECT_BALANCEBOARD)->Enable(RunningWii); if (RunningWii) { + bool was_unpaused = Core::PauseAndLock(true); GetMenuBar()->FindItem(IDM_CONNECT_WIIMOTE1)->Check(GetUsbPointer()-> AccessWiiMote(0x0100)->IsConnected()); GetMenuBar()->FindItem(IDM_CONNECT_WIIMOTE2)->Check(GetUsbPointer()-> @@ -1802,6 +1807,7 @@ void CFrame::UpdateGUI() AccessWiiMote(0x0103)->IsConnected()); GetMenuBar()->FindItem(IDM_CONNECT_BALANCEBOARD)->Check(GetUsbPointer()-> AccessWiiMote(0x0104)->IsConnected()); + Core::PauseAndLock(false, was_unpaused); } if (m_ToolBar) From ba664b3293e88ddee94a4b922ced192f3270b569 Mon Sep 17 00:00:00 2001 From: comex Date: Mon, 13 Apr 2015 02:09:39 -0400 Subject: [PATCH 2/2] Join the emu thread in Core::Stop. Get rid of Core::Shutdown which did that before. Core::Shutdown was only called on app exit, yet the emu thread exits whenever emulation stops; if you launched a new game it would just join via the destructor when s_emu_thread was set to a new thread. (Incidentally, the destructor also makes explicitly joining on app exit rather pointless.) Because the GUI thread wasn't waiting for the CPU thread to fully shut down, Core::IsRunning would remain true briefly after CFrame::DoStop which, given Dolphin's penchant for accessing variables belonging to other threads, can only mean trouble... In my case, because the previous commit caused UpdateGUI, which is called at the end of DoStop, to call PauseAndLock, which checks IsRunning, pressing stop at the right time would cause strange behavior. --- Source/Core/Core/Core.cpp | 8 ++------ Source/Core/Core/Core.h | 1 - Source/Core/DolphinWX/Main.cpp | 1 - Source/Core/DolphinWX/MainNoGUI.cpp | 1 - 4 files changed, 2 insertions(+), 9 deletions(-) diff --git a/Source/Core/Core/Core.cpp b/Source/Core/Core/Core.cpp index 113ac0e553..c3bbaa4515 100644 --- a/Source/Core/Core/Core.cpp +++ b/Source/Core/Core/Core.cpp @@ -267,6 +267,8 @@ void Stop() // - Hammertime! g_video_backend->Video_ExitLoop(); } + if (s_emu_thread.joinable()) + s_emu_thread.join(); } static void DeclareAsCPUThread() @@ -793,12 +795,6 @@ void UpdateTitle() Host_UpdateTitle(SMessage); } -void Shutdown() -{ - if (s_emu_thread.joinable()) - s_emu_thread.join(); -} - void SetOnStoppedCallback(StoppedCallbackFunc callback) { s_on_stopped_callback = callback; diff --git a/Source/Core/Core/Core.h b/Source/Core/Core/Core.h index 341036bead..63eabe247b 100644 --- a/Source/Core/Core/Core.h +++ b/Source/Core/Core/Core.h @@ -40,7 +40,6 @@ enum EState bool Init(); void Stop(); -void Shutdown(); std::string StopMessage(bool, std::string); diff --git a/Source/Core/DolphinWX/Main.cpp b/Source/Core/DolphinWX/Main.cpp index 8734eebedf..300d03720b 100644 --- a/Source/Core/DolphinWX/Main.cpp +++ b/Source/Core/DolphinWX/Main.cpp @@ -371,7 +371,6 @@ void DolphinApp::OnEndSession(wxCloseEvent& event) int DolphinApp::OnExit() { - Core::Shutdown(); UICommon::Shutdown(); delete m_locale; diff --git a/Source/Core/DolphinWX/MainNoGUI.cpp b/Source/Core/DolphinWX/MainNoGUI.cpp index 7e072288fd..0cd7193d22 100644 --- a/Source/Core/DolphinWX/MainNoGUI.cpp +++ b/Source/Core/DolphinWX/MainNoGUI.cpp @@ -343,7 +343,6 @@ int main(int argc, char* argv[]) while (PowerPC::GetState() != PowerPC::CPU_POWERDOWN) updateMainFrameEvent.Wait(); - Core::Shutdown(); platform->Shutdown(); UICommon::Shutdown();