WiimoteReal: Don't use a recursive mutex

This replaces a recursive mutex with a normal mutex.

Source/Core/Core/HW/WiimoteEmu/EmuSubroutines.cpp

@@ -231,7 +231,7 @@ void Wiimote::RequestStatus(const wm_request_status* const rs)
   {
     using namespace WiimoteReal;
 
-    std::lock_guard<std::recursive_mutex> lk(g_refresh_lock);
+    std::lock_guard<std::mutex> lk(g_wiimotes_mutex);
 
     if (g_wiimotes[m_index])
     {

Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.cpp

@@ -662,7 +662,7 @@ void Wiimote::Update()
     {
       using namespace WiimoteReal;
 
-      std::lock_guard<std::recursive_mutex> lk(g_refresh_lock);
+      std::lock_guard<std::mutex> lk(g_wiimotes_mutex);
       if (g_wiimotes[m_index])
       {
         const Report& rpt = g_wiimotes[m_index]->ProcessReadQueue();

Source/Core/Core/HW/WiimoteReal/IOdarwin.mm

@@ -500,7 +500,7 @@ void WiimoteDarwinHid::RemoveCallback(void* context, IOReturn result, void*)
   IOBluetoothDevice* device = [l2capChannel device];
   WiimoteReal::WiimoteDarwin* wm = nullptr;
 
-  std::lock_guard<std::recursive_mutex> lk(WiimoteReal::g_refresh_lock);
+  std::lock_guard<std::mutex> lk(WiimoteReal::g_wiimotes_mutex);
 
   for (int i = 0; i < MAX_WIIMOTES; i++)
   {
@@ -541,7 +541,7 @@ void WiimoteDarwinHid::RemoveCallback(void* context, IOReturn result, void*)
   IOBluetoothDevice* device = [l2capChannel device];
   WiimoteReal::WiimoteDarwin* wm = nullptr;
 
-  std::lock_guard<std::recursive_mutex> lk(WiimoteReal::g_refresh_lock);
+  std::lock_guard<std::mutex> lk(WiimoteReal::g_wiimotes_mutex);
 
   for (int i = 0; i < MAX_WIIMOTES; i++)
   {

Source/Core/Core/HW/WiimoteReal/WiimoteReal.cpp

@@ -25,15 +25,13 @@ unsigned int g_wiimote_sources[MAX_BBMOTES];
 
 namespace WiimoteReal
 {
-void HandleFoundWiimotes(const std::vector<Wiimote*>&);
 void TryToConnectBalanceBoard(Wiimote*);
 void TryToConnectWiimote(Wiimote*);
 void HandleWiimoteDisconnect(int index);
-void DoneWithWiimote(int index);
 
 static bool g_real_wiimotes_initialized = false;
 
-std::recursive_mutex g_refresh_lock;
+std::mutex g_wiimotes_mutex;
 
 Wiimote* g_wiimotes[MAX_BBMOTES];
 WiimoteScanner g_wiimote_scanner;
@@ -402,6 +400,7 @@ void Wiimote::EmuPause()
 
 static unsigned int CalculateConnectedWiimotes()
 {
+  std::lock_guard<std::mutex> lk(g_wiimotes_mutex);
   unsigned int connected_wiimotes = 0;
   for (unsigned int i = 0; i < MAX_WIIMOTES; ++i)
     if (g_wiimotes[i])
@@ -412,6 +411,7 @@ static unsigned int CalculateConnectedWiimotes()
 
 static unsigned int CalculateWantedWiimotes()
 {
+  std::lock_guard<std::mutex> lk(g_wiimotes_mutex);
   // Figure out how many real Wiimotes are required
   unsigned int wanted_wiimotes = 0;
   for (unsigned int i = 0; i < MAX_WIIMOTES; ++i)
@@ -423,6 +423,7 @@ static unsigned int CalculateWantedWiimotes()
 
 static unsigned int CalculateWantedBB()
 {
+  std::lock_guard<std::mutex> lk(g_wiimotes_mutex);
   unsigned int wanted_bb = 0;
   if (WIIMOTE_SRC_REAL & g_wiimote_sources[WIIMOTE_BALANCE_BOARD] &&
       !g_wiimotes[WIIMOTE_BALANCE_BOARD])
@@ -455,8 +456,7 @@ void WiimoteScanner::SetScanMode(WiimoteScanMode scan_mode)
 
 static void CheckForDisconnectedWiimotes()
 {
-  std::lock_guard<std::recursive_mutex> lk(g_refresh_lock);
+  std::lock_guard<std::mutex> lk(g_wiimotes_mutex);
-
   for (unsigned int i = 0; i < MAX_BBMOTES; ++i)
     if (g_wiimotes[i] && !g_wiimotes[i]->IsConnected())
       HandleWiimoteDisconnect(i);
@@ -483,9 +483,12 @@ void WiimoteScanner::ThreadFunc()
       std::vector<Wiimote*> found_wiimotes;
       Wiimote* found_board = nullptr;
       FindWiimotes(found_wiimotes, found_board);
-      HandleFoundWiimotes(found_wiimotes);
+      {
-      if (found_board)
+        std::lock_guard<std::mutex> lk(g_wiimotes_mutex);
-        TryToConnectBalanceBoard(found_board);
+        std::for_each(found_wiimotes.begin(), found_wiimotes.end(), TryToConnectWiimote);
+        if (found_board)
+          TryToConnectBalanceBoard(found_board);
+      }
     }
 
     if (m_scan_mode.load() == WiimoteScanMode::SCAN_ONCE)
@@ -618,8 +621,6 @@ void Initialize(::Wiimote::InitializeMode init_mode)
   else
     g_wiimote_scanner.SetScanMode(WiimoteScanMode::DO_NOT_SCAN);
 
-  std::lock_guard<std::recursive_mutex> lk(g_refresh_lock);
-
   // wait for connection because it should exist before state load
   if (init_mode == ::Wiimote::InitializeMode::DO_WAIT_FOR_WIIMOTES)
   {
@@ -653,10 +654,9 @@ void Shutdown()
 {
   g_wiimote_scanner.StopThread();
 
-  std::lock_guard<std::recursive_mutex> lk(g_refresh_lock);
-
   NOTICE_LOG(WIIMOTE, "WiimoteReal::Shutdown");
 
+  std::lock_guard<std::mutex> lk(g_wiimotes_mutex);
   for (unsigned int i = 0; i < MAX_BBMOTES; ++i)
     HandleWiimoteDisconnect(i);
 }
@@ -677,11 +677,22 @@ void Pause()
 
 void ChangeWiimoteSource(unsigned int index, int source)
 {
+  g_wiimote_sources[index] = source;
   {
-    std::lock_guard<std::recursive_mutex> lk(g_refresh_lock);
-    g_wiimote_sources[index] = source;
     // kill real connection (or swap to different slot)
-    DoneWithWiimote(index);
+    std::lock_guard<std::mutex> lk(g_wiimotes_mutex);
+
+    Wiimote* wm = g_wiimotes[index];
+
+    if (wm)
+    {
+      g_wiimotes[index] = nullptr;
+      // First see if we can use this real Wiimote in another slot.
+      TryToConnectWiimote(wm);
+    }
+
+    // else, just disconnect the Wiimote
+    HandleWiimoteDisconnect(index);
   }
 
   // reconnect to the emulator
@@ -690,7 +701,7 @@ void ChangeWiimoteSource(unsigned int index, int source)
     Host_ConnectWiimote(index, true);
 }
 
-static bool TryToConnectWiimoteN(Wiimote* wm, unsigned int i)
+static bool TryToConnectWiimoteToSlot(Wiimote* wm, unsigned int i)
 {
   if (WIIMOTE_SRC_REAL & g_wiimote_sources[i] && !g_wiimotes[i])
   {
@@ -707,62 +718,30 @@ static bool TryToConnectWiimoteN(Wiimote* wm, unsigned int i)
 
 void TryToConnectWiimote(Wiimote* wm)
 {
-  std::unique_lock<std::recursive_mutex> lk(g_refresh_lock);
-
   for (unsigned int i = 0; i < MAX_WIIMOTES; ++i)
   {
-    if (TryToConnectWiimoteN(wm, i))
+    if (TryToConnectWiimoteToSlot(wm, i))
     {
       wm = nullptr;
       break;
     }
   }
-
-  lk.unlock();
-
   delete wm;
 }
 
 void TryToConnectBalanceBoard(Wiimote* wm)
 {
-  std::unique_lock<std::recursive_mutex> lk(g_refresh_lock);
+  if (TryToConnectWiimoteToSlot(wm, WIIMOTE_BALANCE_BOARD))
-
-  if (TryToConnectWiimoteN(wm, WIIMOTE_BALANCE_BOARD))
   {
     wm = nullptr;
   }
-
-  lk.unlock();
-
   delete wm;
 }
 
-void DoneWithWiimote(int index)
-{
-  std::lock_guard<std::recursive_mutex> lk(g_refresh_lock);
-
-  Wiimote* wm = g_wiimotes[index];
-
-  if (wm)
-  {
-    g_wiimotes[index] = nullptr;
-    // First see if we can use this real Wiimote in another slot.
-    TryToConnectWiimote(wm);
-  }
-
-  // else, just disconnect the Wiimote
-  HandleWiimoteDisconnect(index);
-}
-
 void HandleWiimoteDisconnect(int index)
 {
   Wiimote* wm = nullptr;
-
+  std::swap(wm, g_wiimotes[index]);
-  {
-    std::lock_guard<std::recursive_mutex> lk(g_refresh_lock);
-    std::swap(wm, g_wiimotes[index]);
-  }
-
   if (wm)
   {
     delete wm;
@@ -770,11 +749,6 @@ void HandleWiimoteDisconnect(int index)
   }
 }
 
-void HandleFoundWiimotes(const std::vector<Wiimote*>& wiimotes)
-{
-  std::for_each(wiimotes.begin(), wiimotes.end(), TryToConnectWiimote);
-}
-
 // This is called from the GUI thread
 void Refresh()
 {
@@ -784,54 +758,51 @@ void Refresh()
 
 void InterruptChannel(int _WiimoteNumber, u16 _channelID, const void* _pData, u32 _Size)
 {
-  std::lock_guard<std::recursive_mutex> lk(g_refresh_lock);
+  std::lock_guard<std::mutex> lk(g_wiimotes_mutex);
   if (g_wiimotes[_WiimoteNumber])
     g_wiimotes[_WiimoteNumber]->InterruptChannel(_channelID, _pData, _Size);
 }
 
 void ControlChannel(int _WiimoteNumber, u16 _channelID, const void* _pData, u32 _Size)
 {
-  std::lock_guard<std::recursive_mutex> lk(g_refresh_lock);
+  std::lock_guard<std::mutex> lk(g_wiimotes_mutex);
-
   if (g_wiimotes[_WiimoteNumber])
     g_wiimotes[_WiimoteNumber]->ControlChannel(_channelID, _pData, _Size);
 }
 
 // Read the Wiimote once
-void Update(int _WiimoteNumber)
+void Update(int wiimote_number)
 {
   // Try to get a lock and return without doing anything if we fail
-  // This avoids deadlocks when adding a Wiimote during continuous scan
+  // This avoids blocking the CPU thread
-  if (!g_refresh_lock.try_lock())
+  if (!g_wiimotes_mutex.try_lock())
     return;
 
-  if (g_wiimotes[_WiimoteNumber])
+  if (g_wiimotes[wiimote_number])
-    g_wiimotes[_WiimoteNumber]->Update();
+    g_wiimotes[wiimote_number]->Update();
 
   // Wiimote::Update() may remove the Wiimote if it was disconnected.
-  if (!g_wiimotes[_WiimoteNumber])
+  if (!g_wiimotes[wiimote_number])
   {
-    Host_ConnectWiimote(_WiimoteNumber, false);
+    Host_ConnectWiimote(wiimote_number, false);
   }
-  g_refresh_lock.unlock();
+
+  g_wiimotes_mutex.unlock();
 }
 
-void ConnectOnInput(int _WiimoteNumber)
+void ConnectOnInput(int wiimote_number)
 {
-  // see Update() above
+  if (!g_wiimotes_mutex.try_lock())
-  if (!g_refresh_lock.try_lock())
     return;
 
-  if (g_wiimotes[_WiimoteNumber])
+  if (g_wiimotes[wiimote_number])
-    g_wiimotes[_WiimoteNumber]->ConnectOnInput();
+    g_wiimotes[wiimote_number]->ConnectOnInput();
 
-  g_refresh_lock.unlock();
+  g_wiimotes_mutex.unlock();
 }
 
 void StateChange(EMUSTATE_CHANGE newState)
 {
-  // std::lock_guard<std::recursive_mutex> lk(g_refresh_lock);
-
   // TODO: disable/enable auto reporting, maybe
 }
 

Source/Core/Core/HW/WiimoteReal/WiimoteReal.h

@@ -156,7 +156,7 @@ private:
 #endif
 };
 
-extern std::recursive_mutex g_refresh_lock;
+extern std::mutex g_wiimotes_mutex;
 extern WiimoteScanner g_wiimote_scanner;
 extern Wiimote* g_wiimotes[MAX_BBMOTES];