From d4cab35533fe5a5f8bf609d9209655ecff5ed749 Mon Sep 17 00:00:00 2001 From: Rodrigo Locatti Date: Tue, 22 Jun 2021 04:30:07 -0300 Subject: [PATCH] input_common: Fix data race on GC implementation --- src/input_common/gcadapter/gc_adapter.cpp | 189 ++++++++++++---------- src/input_common/gcadapter/gc_adapter.h | 46 +++--- 2 files changed, 120 insertions(+), 115 deletions(-) diff --git a/src/input_common/gcadapter/gc_adapter.cpp b/src/input_common/gcadapter/gc_adapter.cpp index a2f1bb67c8..3c3b6fbde0 100644 --- a/src/input_common/gcadapter/gc_adapter.cpp +++ b/src/input_common/gcadapter/gc_adapter.cpp @@ -14,15 +14,73 @@ namespace GCAdapter { +class LibUSBContext { +public: + explicit LibUSBContext() { + init_result = libusb_init(&ctx); + } + + ~LibUSBContext() { + libusb_exit(ctx); + } + + LibUSBContext& operator=(const LibUSBContext&) = delete; + LibUSBContext(const LibUSBContext&) = delete; + + LibUSBContext& operator=(LibUSBContext&&) noexcept = delete; + LibUSBContext(LibUSBContext&&) noexcept = delete; + + [[nodiscard]] int InitResult() const noexcept { + return init_result; + } + + [[nodiscard]] libusb_context* get() noexcept { + return ctx; + } + +private: + libusb_context* ctx; + int init_result{}; +}; + +class LibUSBDeviceHandle { +public: + explicit LibUSBDeviceHandle(libusb_context* ctx, uint16_t vid, uint16_t pid) noexcept { + handle = libusb_open_device_with_vid_pid(ctx, vid, pid); + } + + ~LibUSBDeviceHandle() noexcept { + if (handle) { + libusb_release_interface(handle, 1); + libusb_close(handle); + } + } + + LibUSBDeviceHandle& operator=(const LibUSBDeviceHandle&) = delete; + LibUSBDeviceHandle(const LibUSBDeviceHandle&) = delete; + + LibUSBDeviceHandle& operator=(LibUSBDeviceHandle&&) noexcept = delete; + LibUSBDeviceHandle(LibUSBDeviceHandle&&) noexcept = delete; + + [[nodiscard]] libusb_device_handle* get() noexcept { + return handle; + } + +private: + libusb_device_handle* handle{}; +}; + Adapter::Adapter() { - if (usb_adapter_handle != nullptr) { + if (usb_adapter_handle) { return; } LOG_INFO(Input, "GC Adapter Initialization started"); - const int init_res = libusb_init(&libusb_ctx); + libusb_ctx = std::make_unique(); + const int init_res = libusb_ctx->InitResult(); if (init_res == LIBUSB_SUCCESS) { - adapter_scan_thread = std::thread(&Adapter::AdapterScanThread, this); + adapter_scan_thread = + std::jthread([this](std::stop_token stop_token) { AdapterScanThread(stop_token); }); } else { LOG_ERROR(Input, "libusb could not be initialized. failed with error = {}", init_res); } @@ -32,17 +90,15 @@ Adapter::~Adapter() { Reset(); } -void Adapter::AdapterInputThread() { +void Adapter::AdapterInputThread(std::stop_token stop_token) { LOG_DEBUG(Input, "GC Adapter input thread started"); s32 payload_size{}; AdapterPayload adapter_payload{}; - if (adapter_scan_thread.joinable()) { - adapter_scan_thread.join(); - } + adapter_scan_thread = {}; - while (adapter_input_thread_running) { - libusb_interrupt_transfer(usb_adapter_handle, input_endpoint, adapter_payload.data(), + while (!stop_token.stop_requested()) { + libusb_interrupt_transfer(usb_adapter_handle->get(), input_endpoint, adapter_payload.data(), static_cast(adapter_payload.size()), &payload_size, 16); if (IsPayloadCorrect(adapter_payload, payload_size)) { UpdateControllers(adapter_payload); @@ -52,7 +108,8 @@ void Adapter::AdapterInputThread() { } if (restart_scan_thread) { - adapter_scan_thread = std::thread(&Adapter::AdapterScanThread, this); + adapter_scan_thread = + std::jthread([this](std::stop_token token) { AdapterScanThread(token); }); restart_scan_thread = false; } } @@ -64,7 +121,7 @@ bool Adapter::IsPayloadCorrect(const AdapterPayload& adapter_payload, s32 payloa adapter_payload[0]); if (input_error_counter++ > 20) { LOG_ERROR(Input, "GC adapter timeout, Is the adapter connected?"); - adapter_input_thread_running = false; + adapter_input_thread.request_stop(); restart_scan_thread = true; } return false; @@ -96,7 +153,7 @@ void Adapter::UpdatePadType(std::size_t port, ControllerTypes pad_type) { return; } // Device changed reset device and set new type - ResetDevice(port); + pads[port] = {}; pads[port].type = pad_type; } @@ -213,8 +270,9 @@ void Adapter::SendVibrations() { const u8 p3 = pads[2].enable_vibration; const u8 p4 = pads[3].enable_vibration; std::array payload = {rumble_command, p1, p2, p3, p4}; - const int err = libusb_interrupt_transfer(usb_adapter_handle, output_endpoint, payload.data(), - static_cast(payload.size()), &size, 16); + const int err = + libusb_interrupt_transfer(usb_adapter_handle->get(), output_endpoint, payload.data(), + static_cast(payload.size()), &size, 16); if (err) { LOG_DEBUG(Input, "Adapter libusb write failed: {}", libusb_error_name(err)); if (output_error_counter++ > 5) { @@ -233,56 +291,53 @@ bool Adapter::RumblePlay(std::size_t port, u8 amplitude) { return rumble_enabled; } -void Adapter::AdapterScanThread() { - adapter_scan_thread_running = true; - adapter_input_thread_running = false; - if (adapter_input_thread.joinable()) { - adapter_input_thread.join(); - } - ClearLibusbHandle(); - ResetDevices(); - while (adapter_scan_thread_running && !adapter_input_thread_running) { - Setup(); - std::this_thread::sleep_for(std::chrono::seconds(1)); +void Adapter::AdapterScanThread(std::stop_token stop_token) { + usb_adapter_handle = nullptr; + pads = {}; + while (!stop_token.stop_requested() && !Setup()) { + std::this_thread::sleep_for(std::chrono::seconds(2)); } } -void Adapter::Setup() { - usb_adapter_handle = libusb_open_device_with_vid_pid(libusb_ctx, 0x057e, 0x0337); - - if (usb_adapter_handle == NULL) { - return; +bool Adapter::Setup() { + constexpr u16 nintendo_vid = 0x057e; + constexpr u16 gc_adapter_pid = 0x0337; + usb_adapter_handle = + std::make_unique(libusb_ctx->get(), nintendo_vid, gc_adapter_pid); + if (!usb_adapter_handle->get()) { + return false; } if (!CheckDeviceAccess()) { - ClearLibusbHandle(); - return; + usb_adapter_handle = nullptr; + return false; } - libusb_device* device = libusb_get_device(usb_adapter_handle); + libusb_device* const device = libusb_get_device(usb_adapter_handle->get()); LOG_INFO(Input, "GC adapter is now connected"); // GC Adapter found and accessible, registering it if (GetGCEndpoint(device)) { - adapter_scan_thread_running = false; - adapter_input_thread_running = true; rumble_enabled = true; input_error_counter = 0; output_error_counter = 0; - adapter_input_thread = std::thread(&Adapter::AdapterInputThread, this); + adapter_input_thread = + std::jthread([this](std::stop_token stop_token) { AdapterInputThread(stop_token); }); + return true; } + return false; } bool Adapter::CheckDeviceAccess() { // This fixes payload problems from offbrand GCAdapters const s32 control_transfer_error = - libusb_control_transfer(usb_adapter_handle, 0x21, 11, 0x0001, 0, nullptr, 0, 1000); + libusb_control_transfer(usb_adapter_handle->get(), 0x21, 11, 0x0001, 0, nullptr, 0, 1000); if (control_transfer_error < 0) { LOG_ERROR(Input, "libusb_control_transfer failed with error= {}", control_transfer_error); } - s32 kernel_driver_error = libusb_kernel_driver_active(usb_adapter_handle, 0); + s32 kernel_driver_error = libusb_kernel_driver_active(usb_adapter_handle->get(), 0); if (kernel_driver_error == 1) { - kernel_driver_error = libusb_detach_kernel_driver(usb_adapter_handle, 0); + kernel_driver_error = libusb_detach_kernel_driver(usb_adapter_handle->get(), 0); if (kernel_driver_error != 0 && kernel_driver_error != LIBUSB_ERROR_NOT_SUPPORTED) { LOG_ERROR(Input, "libusb_detach_kernel_driver failed with error = {}", kernel_driver_error); @@ -290,15 +345,13 @@ bool Adapter::CheckDeviceAccess() { } if (kernel_driver_error && kernel_driver_error != LIBUSB_ERROR_NOT_SUPPORTED) { - libusb_close(usb_adapter_handle); usb_adapter_handle = nullptr; return false; } - const int interface_claim_error = libusb_claim_interface(usb_adapter_handle, 0); + const int interface_claim_error = libusb_claim_interface(usb_adapter_handle->get(), 0); if (interface_claim_error) { LOG_ERROR(Input, "libusb_claim_interface failed with error = {}", interface_claim_error); - libusb_close(usb_adapter_handle); usb_adapter_handle = nullptr; return false; } @@ -332,57 +385,17 @@ bool Adapter::GetGCEndpoint(libusb_device* device) { // This transfer seems to be responsible for clearing the state of the adapter // Used to clear the "busy" state of when the device is unexpectedly unplugged unsigned char clear_payload = 0x13; - libusb_interrupt_transfer(usb_adapter_handle, output_endpoint, &clear_payload, + libusb_interrupt_transfer(usb_adapter_handle->get(), output_endpoint, &clear_payload, sizeof(clear_payload), nullptr, 16); return true; } -void Adapter::JoinThreads() { - restart_scan_thread = false; - adapter_input_thread_running = false; - adapter_scan_thread_running = false; - - if (adapter_scan_thread.joinable()) { - adapter_scan_thread.join(); - } - - if (adapter_input_thread.joinable()) { - adapter_input_thread.join(); - } -} - -void Adapter::ClearLibusbHandle() { - if (usb_adapter_handle) { - libusb_release_interface(usb_adapter_handle, 1); - libusb_close(usb_adapter_handle); - usb_adapter_handle = nullptr; - } -} - -void Adapter::ResetDevices() { - for (std::size_t i = 0; i < pads.size(); ++i) { - ResetDevice(i); - } -} - -void Adapter::ResetDevice(std::size_t port) { - pads[port].type = ControllerTypes::None; - pads[port].enable_vibration = false; - pads[port].rumble_amplitude = 0; - pads[port].buttons = 0; - pads[port].last_button = PadButton::Undefined; - pads[port].axis_values.fill(0); - pads[port].reset_origin_counter = 0; -} - void Adapter::Reset() { - JoinThreads(); - ClearLibusbHandle(); - ResetDevices(); - - if (libusb_ctx) { - libusb_exit(libusb_ctx); - } + adapter_scan_thread = {}; + adapter_input_thread = {}; + usb_adapter_handle = nullptr; + pads = {}; + libusb_ctx = nullptr; } std::vector Adapter::GetInputDevices() const { diff --git a/src/input_common/gcadapter/gc_adapter.h b/src/input_common/gcadapter/gc_adapter.h index e5de5e94fc..28dbcbe05f 100644 --- a/src/input_common/gcadapter/gc_adapter.h +++ b/src/input_common/gcadapter/gc_adapter.h @@ -3,11 +3,14 @@ // Refer to the license.txt file included. #pragma once + #include #include #include +#include #include #include + #include "common/common_types.h" #include "common/threadsafe_queue.h" #include "input_common/main.h" @@ -18,6 +21,9 @@ struct libusb_device_handle; namespace GCAdapter { +class LibUSBContext; +class LibUSBDeviceHandle; + enum class PadButton { Undefined = 0x0000, ButtonLeft = 0x0001, @@ -63,11 +69,11 @@ struct GCPadStatus { }; struct GCController { - ControllerTypes type{}; - bool enable_vibration{}; - u8 rumble_amplitude{}; - u16 buttons{}; - PadButton last_button{}; + ControllerTypes type = ControllerTypes::None; + bool enable_vibration = false; + u8 rumble_amplitude = 0; + u16 buttons = 0; + PadButton last_button = PadButton::Undefined; std::array axis_values{}; std::array axis_origin{}; u8 reset_origin_counter{}; @@ -109,9 +115,9 @@ private: void UpdateStateAxes(std::size_t port, const AdapterPayload& adapter_payload); void UpdateVibrations(); - void AdapterInputThread(); + void AdapterInputThread(std::stop_token stop_token); - void AdapterScanThread(); + void AdapterScanThread(std::stop_token stop_token); bool IsPayloadCorrect(const AdapterPayload& adapter_payload, s32 payload_size); @@ -119,13 +125,7 @@ private: void SendVibrations(); /// For use in initialization, querying devices to find the adapter - void Setup(); - - /// Resets status of all GC controller devices to a disconnected state - void ResetDevices(); - - /// Resets status of device connected to a disconnected state - void ResetDevice(std::size_t port); + bool Setup(); /// Returns true if we successfully gain access to GC Adapter bool CheckDeviceAccess(); @@ -137,23 +137,15 @@ private: /// For shutting down, clear all data, join all threads, release usb void Reset(); - // Join all threads - void JoinThreads(); - - // Release usb handles - void ClearLibusbHandle(); - - libusb_device_handle* usb_adapter_handle = nullptr; + std::unique_ptr usb_adapter_handle; std::array pads; Common::SPSCQueue pad_queue; - std::thread adapter_input_thread; - std::thread adapter_scan_thread; - bool adapter_input_thread_running; - bool adapter_scan_thread_running; - bool restart_scan_thread; + std::jthread adapter_input_thread; + std::jthread adapter_scan_thread; + bool restart_scan_thread{}; - libusb_context* libusb_ctx; + std::unique_ptr libusb_ctx; u8 input_endpoint{0}; u8 output_endpoint{0};