diff --git a/Source/Core/Core/HW/GCKeyboardEmu.cpp b/Source/Core/Core/HW/GCKeyboardEmu.cpp index 23e49898fc..b022527a0d 100644 --- a/Source/Core/Core/HW/GCKeyboardEmu.cpp +++ b/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); diff --git a/Source/Core/Core/HW/GCPadEmu.cpp b/Source/Core/Core/HW/GCPadEmu.cpp index f985342b62..36ab385486 100644 --- a/Source/Core/Core/HW/GCPadEmu.cpp +++ b/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()); } diff --git a/Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.cpp b/Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.cpp index 276943b578..b9dec6f554 100644 --- a/Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.cpp +++ b/Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.cpp @@ -623,8 +623,11 @@ void Wiimote::Update() return; // returns true if a report was sent - if (Step()) - return; + { + 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); diff --git a/Source/Core/Core/HotkeyManager.cpp b/Source/Core/Core/HotkeyManager.cpp index cad32b65b9..b8fc387b3e 100644 --- a/Source/Core/Core/HotkeyManager.cpp +++ b/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 bitmasks; diff --git a/Source/Core/DolphinWX/InputConfigDiag.cpp b/Source/Core/DolphinWX/InputConfigDiag.cpp index 2a5aa997e3..cad5d3496a 100644 --- a/Source/Core/DolphinWX/InputConfigDiag.cpp +++ b/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; } diff --git a/Source/Core/DolphinWX/InputConfigDiagBitmaps.cpp b/Source/Core/DolphinWX/InputConfigDiagBitmaps.cpp index 13dcea4e2f..8f972c9091 100644 --- a/Source/Core/DolphinWX/InputConfigDiagBitmaps.cpp +++ b/Source/Core/DolphinWX/InputConfigDiagBitmaps.cpp @@ -4,7 +4,6 @@ #include #include -#include #include #include #include @@ -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,17 +232,15 @@ 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(); - yy = g->control_group->controls[1]->control_ref->State(); - yy -= g->control_group->controls[0]->control_ref->State(); + ControlState xx, yy; + xx = g->control_group->controls[3]->control_ref->State(); + xx -= g->control_group->controls[2]->control_ref->State(); + yy = g->control_group->controls[1]->control_ref->State(); + yy -= g->control_group->controls[0]->control_ref->State(); - dc.SetPen(*wxGREY_PEN); - dc.SetBrush(*wxGREY_BRUSH); - DrawCoordinate(dc, xx, yy); - } + 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 diff --git a/Source/Core/InputCommon/ControllerEmu.cpp b/Source/Core/InputCommon/ControllerEmu.cpp index 117184c8db..b3b13b796a 100644 --- a/Source/Core/InputCommon/ControllerEmu.cpp +++ b/Source/Core/InputCommon/ControllerEmu.cpp @@ -6,8 +6,19 @@ #include #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 ControllerEmu::GetStateLock() +{ + std::unique_lock 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) diff --git a/Source/Core/InputCommon/ControllerEmu.h b/Source/Core/InputCommon/ControllerEmu.h index d9470ca8a9..9416b5f0de 100644 --- a/Source/Core/InputCommon/ControllerEmu.h +++ b/Source/Core/InputCommon/ControllerEmu.h @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -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 GetStateLock(); + std::vector> groups; ciface::Core::DeviceQualifier default_device; diff --git a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp index cc80b7b234..cf726698f1 100644 --- a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp +++ b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.cpp @@ -160,6 +160,14 @@ void ControllerInterface::AddDevice(std::shared_ptr device m_devices.emplace_back(std::move(device)); } +void ControllerInterface::RemoveDevice(std::function callback) +{ + std::lock_guard 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 // diff --git a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h index 24f92fd3c1..198a2c180b 100644 --- a/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h +++ b/Source/Core/InputCommon/ControllerInterface/ControllerInterface.h @@ -122,6 +122,7 @@ public: void Reinitialize(); void Shutdown(); void AddDevice(std::shared_ptr device); + void RemoveDevice(std::function callback); bool IsInit() const { return m_is_init; } void UpdateReference(ControlReference* control, const ciface::Core::DeviceQualifier& default_device) const; diff --git a/Source/Core/InputCommon/ControllerInterface/Device.h b/Source/Core/InputCommon/ControllerInterface/Device.h index e3fe4c11cd..657ece5016 100644 --- a/Source/Core/InputCommon/ControllerInterface/Device.h +++ b/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& Inputs() const { return m_inputs; } const std::vector& Outputs() const { return m_outputs; } Input* FindInput(const std::string& name) const; diff --git a/Source/Core/InputCommon/ControllerInterface/ExpressionParser.cpp b/Source/Core/InputCommon/ControllerInterface/ExpressionParser.cpp index 6e9b8b77ab..2960888148 100644 --- a/Source/Core/InputCommon/ControllerInterface/ExpressionParser.cpp +++ b/Source/Core/InputCommon/ControllerInterface/ExpressionParser.cpp @@ -235,8 +235,9 @@ public: ControlQualifier qualifier; Device::Control* control; - ControlExpression(ControlQualifier qualifier_, Device::Control* control_) - : qualifier(qualifier_), control(control_) + ControlExpression(ControlQualifier qualifier_, std::shared_ptr device, + 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 m_device; }; class BinaryExpression : public ExpressionNode @@ -393,6 +396,7 @@ private: { case TOK_CONTROL: { + std::shared_ptr 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 = 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; } diff --git a/Source/Core/InputCommon/ControllerInterface/ExpressionParser.h b/Source/Core/InputCommon/ControllerInterface/ExpressionParser.h index 992ca65f03..2c8f5a793f 100644 --- a/Source/Core/InputCommon/ControllerInterface/ExpressionParser.h +++ b/Source/Core/InputCommon/ControllerInterface/ExpressionParser.h @@ -37,10 +37,10 @@ public: : container(container_), default_device(default_), is_input(is_input_) { } + std::shared_ptr FindDevice(ControlQualifier qualifier); Core::Device::Control* FindControl(ControlQualifier qualifier); private: - std::shared_ptr FindDevice(ControlQualifier qualifier); const Core::DeviceContainer& container; const Core::DeviceQualifier& default_device; bool is_input;