ControllerInterface: Add RemoveDevice()

This adds RemoveDevice() to ControllerInterface, fixes ExpressionParser
and some other code to support device removals without crashing,
and adds an IsValid() method to Device, to prepare for hotplugging.

Source/Core/Core/HW/GCKeyboardEmu.cpp

@@ -86,6 +86,7 @@ std::string GCKeyboard::GetName() const
 
 void GCKeyboard::GetInput(KeyboardStatus* const kb)
 {
+  auto lock = ControllerEmu::GetStateLock();
   m_keys0x->GetState(&kb->key0x, keys0_bitmasks);
   m_keys1x->GetState(&kb->key1x, keys1_bitmasks);
   m_keys2x->GetState(&kb->key2x, keys2_bitmasks);

Source/Core/Core/HW/GCPadEmu.cpp

@@ -81,6 +81,8 @@ std::string GCPad::GetName() const
 
 void GCPad::GetInput(GCPadStatus* const pad)
 {
+  auto lock = ControllerEmu::GetStateLock();
+
   ControlState x, y, triggers[2];
 
   // buttons
@@ -116,6 +118,7 @@ void GCPad::GetInput(GCPadStatus* const pad)
 
 void GCPad::SetOutput(const ControlState strength)
 {
+  auto lock = ControllerEmu::GetStateLock();
   m_rumble->controls[0]->control_ref->State(strength);
 }
 
@@ -190,5 +193,6 @@ void GCPad::LoadDefaults(const ControllerInterface& ciface)
 
 bool GCPad::GetMicButton() const
 {
+  auto lock = ControllerEmu::GetStateLock();
   return (0.0f != m_buttons->controls.back()->control_ref->State());
 }

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

@@ -623,8 +623,11 @@ void Wiimote::Update()
     return;
 
   // returns true if a report was sent
+  {
+    auto lock = ControllerEmu::GetStateLock();
     if (Step())
       return;
+  }
 
   u8 data[MAX_PAYLOAD];
   memset(data, 0, sizeof(data));
@@ -646,6 +649,8 @@ void Wiimote::Update()
     data[0] = 0xA1;
     data[1] = m_reporting_mode;
 
+    auto lock = ControllerEmu::GetStateLock();
+
     // core buttons
     if (rptf.core)
       GetButtonData(data + rptf.core);
@@ -876,6 +881,7 @@ void Wiimote::ConnectOnInput()
   }
 
   u16 buttons = 0;
+  auto lock = ControllerEmu::GetStateLock();
   m_buttons->GetState(&buttons, button_bitmasks);
   m_dpad->GetState(&buttons, dpad_bitmasks);
 

Source/Core/Core/HotkeyManager.cpp

@@ -240,6 +240,7 @@ std::string HotkeyManager::GetName() const
 
 void HotkeyManager::GetInput(HotkeyStatus* const kb)
 {
+  auto lock = ControllerEmu::GetStateLock();
   for (int set = 0; set < (NUM_HOTKEYS + 31) / 32; set++)
   {
     std::vector<u32> bitmasks;

Source/Core/DolphinWX/InputConfigDiag.cpp

@@ -315,11 +315,12 @@ bool ControlDialog::Validate()
 {
   control_reference->expression = WxStrToStr(textctrl->GetValue());
 
+  auto lock = ControllerEmu::GetStateLock();
   g_controller_interface.UpdateReference(control_reference, m_parent->controller->default_device);
 
   UpdateGUI();
 
-  return (control_reference->parse_error == EXPRESSION_PARSE_SUCCESS);
+  return control_reference->parse_error == EXPRESSION_PARSE_SUCCESS;
 }
 
 void GamepadPage::SetDevice(wxCommandEvent&)
@@ -351,6 +352,7 @@ void ControlDialog::ClearControl(wxCommandEvent&)
 {
   control_reference->expression.clear();
 
+  auto lock = ControllerEmu::GetStateLock();
   g_controller_interface.UpdateReference(control_reference, m_parent->controller->default_device);
 
   UpdateGUI();
@@ -408,6 +410,7 @@ void ControlDialog::SetSelectedControl(wxCommandEvent&)
   textctrl->WriteText(expr);
   control_reference->expression = textctrl->GetValue();
 
+  auto lock = ControllerEmu::GetStateLock();
   g_controller_interface.UpdateReference(control_reference, m_parent->controller->default_device);
 
   UpdateGUI();
@@ -442,6 +445,7 @@ void ControlDialog::AppendControl(wxCommandEvent& event)
   textctrl->WriteText(expr);
   control_reference->expression = textctrl->GetValue();
 
+  auto lock = ControllerEmu::GetStateLock();
   g_controller_interface.UpdateReference(control_reference, m_parent->controller->default_device);
 
   UpdateGUI();
@@ -556,6 +560,7 @@ bool GamepadPage::DetectButton(ControlButton* button)
       wxString expr;
       GetExpressionForControl(expr, control_name);
       button->control_reference->expression = expr;
+      auto lock = ControllerEmu::GetStateLock();
       g_controller_interface.UpdateReference(button->control_reference, controller->default_device);
       success = true;
     }

Source/Core/DolphinWX/InputConfigDiagBitmaps.cpp

@@ -4,7 +4,6 @@
 
 #include <cstring>
 #include <memory>
-#include <mutex>
 #include <string>
 #include <vector>
 #include <wx/bitmap.h>
@@ -125,6 +124,7 @@ static void DrawButton(unsigned int* const bitmasks, unsigned int buttons, unsig
   }
   else
   {
+    auto lock = ControllerEmu::GetStateLock();
     unsigned char amt = 255 - g->control_group->controls[(row * 8) + n]->control_ref->State() * 128;
     dc.SetBrush(wxBrush(wxColour(amt, amt, amt)));
   }
@@ -232,7 +232,6 @@ static void DrawControlGroupBox(wxDC& dc, ControlGroupBox* g)
     }
 
     // raw dot
-    {
     ControlState xx, yy;
     xx = g->control_group->controls[3]->control_ref->State();
     xx -= g->control_group->controls[2]->control_ref->State();
@@ -242,7 +241,6 @@ static void DrawControlGroupBox(wxDC& dc, ControlGroupBox* g)
     dc.SetPen(*wxGREY_PEN);
     dc.SetBrush(*wxGREY_BRUSH);
     DrawCoordinate(dc, xx, yy);
-    }
 
     // adjusted dot
     if (x != 0 || y != 0)
@@ -403,6 +401,7 @@ static void DrawControlGroupBox(wxDC& dc, ControlGroupBox* g)
     for (unsigned int n = 0; n < trigger_count; ++n)
     {
       dc.SetBrush(*wxRED_BRUSH);
+
       ControlState trig_d = g->control_group->controls[n]->control_ref->State();
 
       ControlState trig_a =
@@ -465,6 +464,7 @@ void InputConfigDialog::UpdateBitmaps(wxTimerEvent& WXUNUSED(event))
   GamepadPage* const current_page =
       (GamepadPage*)m_pad_notebook->GetPage(m_pad_notebook->GetSelection());
 
+  auto lock = ControllerEmu::GetStateLock();
   for (ControlGroupBox* g : current_page->control_groups)
   {
     // if this control group has a bitmap

Source/Core/InputCommon/ControllerEmu.cpp

@@ -6,8 +6,19 @@
 #include <memory>
 #include "Common/Common.h"
 
+// This should be called before calling GetState() or State() on a control reference
+// to prevent a race condition.
+// This is a recursive mutex because UpdateReferences is recursive.
+static std::recursive_mutex s_get_state_mutex;
+std::unique_lock<std::recursive_mutex> ControllerEmu::GetStateLock()
+{
+  std::unique_lock<std::recursive_mutex> lock(s_get_state_mutex);
+  return lock;
+}
+
 void ControllerEmu::UpdateReferences(ControllerInterface& devi)
 {
+  auto lock = ControllerEmu::GetStateLock();
   for (auto& ctrlGroup : groups)
   {
     for (auto& control : ctrlGroup->controls)

Source/Core/InputCommon/ControllerEmu.h

@@ -7,6 +7,7 @@
 #include <algorithm>
 #include <cmath>
 #include <memory>
+#include <mutex>
 #include <string>
 #include <vector>
 
@@ -444,6 +445,12 @@ public:
 
   void UpdateReferences(ControllerInterface& devi);
 
+  // This returns a lock that should be held before calling State() on any control
+  // references and GetState(), by extension. This prevents a race condition
+  // which happens while handling a hotplug event because a control reference's State()
+  // could be called before we have finished updating the reference.
+  static std::unique_lock<std::recursive_mutex> GetStateLock();
+
   std::vector<std::unique_ptr<ControlGroup>> groups;
 
   ciface::Core::DeviceQualifier default_device;

Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp

@@ -160,6 +160,14 @@ void ControllerInterface::AddDevice(std::shared_ptr<ciface::Core::Device> device
   m_devices.emplace_back(std::move(device));
 }
 
+void ControllerInterface::RemoveDevice(std::function<bool(const ciface::Core::Device*)> callback)
+{
+  std::lock_guard<std::mutex> lk(m_devices_mutex);
+  m_devices.erase(std::remove_if(m_devices.begin(), m_devices.end(),
+                                 [&callback](const auto& dev) { return callback(dev.get()); }),
+                  m_devices.end());
+}
+
 //
 // UpdateInput
 //

Source/Core/InputCommon/ControllerInterface/ControllerInterface.h

@@ -122,6 +122,7 @@ public:
   void Reinitialize();
   void Shutdown();
   void AddDevice(std::shared_ptr<ciface::Core::Device> device);
+  void RemoveDevice(std::function<bool(const ciface::Core::Device*)> callback);
   bool IsInit() const { return m_is_init; }
   void UpdateReference(ControlReference* control,
                        const ciface::Core::DeviceQualifier& default_device) const;

Source/Core/InputCommon/ControllerInterface/Device.h

@@ -98,6 +98,7 @@ public:
   virtual std::string GetName() const = 0;
   virtual std::string GetSource() const = 0;
   virtual void UpdateInput() {}
+  virtual bool IsValid() const { return true; }
   const std::vector<Input*>& Inputs() const { return m_inputs; }
   const std::vector<Output*>& Outputs() const { return m_outputs; }
   Input* FindInput(const std::string& name) const;

Source/Core/InputCommon/ControllerInterface/ExpressionParser.cpp

@@ -235,8 +235,9 @@ public:
   ControlQualifier qualifier;
   Device::Control* control;
 
-  ControlExpression(ControlQualifier qualifier_, Device::Control* control_)
+  ControlExpression(ControlQualifier qualifier_, std::shared_ptr<Device> device,
-      : qualifier(qualifier_), control(control_)
+                    Device::Control* control_)
+      : qualifier(qualifier_), control(control_), m_device(device)
   {
   }
 
@@ -244,6 +245,8 @@ public:
   void SetValue(ControlState value) override { control->ToOutput()->SetGatedState(value); }
   int CountNumControls() override { return 1; }
   operator std::string() override { return "`" + (std::string)qualifier + "`"; }
+private:
+  std::shared_ptr<Device> m_device;
 };
 
 class BinaryExpression : public ExpressionNode
@@ -393,6 +396,7 @@ private:
     {
     case TOK_CONTROL:
     {
+      std::shared_ptr<Device> device = finder.FindDevice(tok.qualifier);
       Device::Control* control = finder.FindControl(tok.qualifier);
       if (control == nullptr)
       {
@@ -400,7 +404,7 @@ private:
         return EXPRESSION_PARSE_SUCCESS;
       }
 
-      *expr_out = new ControlExpression(tok.qualifier, control);
+      *expr_out = new ControlExpression(tok.qualifier, device, control);
       return EXPRESSION_PARSE_SUCCESS;
     }
     case TOK_LPAREN:
@@ -550,10 +554,11 @@ ExpressionParseStatus ParseExpression(const std::string& str, ControlFinder& fin
   qualifier.control_name = str;
   qualifier.has_device = false;
 
+  std::shared_ptr<Device> device = finder.FindDevice(qualifier);
   Device::Control* control = finder.FindControl(qualifier);
   if (control)
   {
-    *expr_out = new Expression(new ControlExpression(qualifier, control));
+    *expr_out = new Expression(new ControlExpression(qualifier, device, control));
     return EXPRESSION_PARSE_SUCCESS;
   }
 

Source/Core/InputCommon/ControllerInterface/ExpressionParser.h

@@ -37,10 +37,10 @@ public:
       : container(container_), default_device(default_), is_input(is_input_)
   {
   }
+  std::shared_ptr<Core::Device> FindDevice(ControlQualifier qualifier);
   Core::Device::Control* FindControl(ControlQualifier qualifier);
 
 private:
-  std::shared_ptr<Core::Device> FindDevice(ControlQualifier qualifier);
   const Core::DeviceContainer& container;
   const Core::DeviceQualifier& default_device;
   bool is_input;