Merge pull request #7217 from yuzu-emu/revert-6515-gc_thread_safe

Revert "input_common: Fix data race on GC implementation"

src/input_common/gcadapter/gc_adapter.cpp

@@ -14,73 +14,15 @@
 
 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) {
+    if (usb_adapter_handle != nullptr) {
         return;
     }
     LOG_INFO(Input, "GC Adapter Initialization started");
 
-    libusb_ctx = std::make_unique<LibUSBContext>();
+    const int init_res = libusb_init(&libusb_ctx);
-    const int init_res = libusb_ctx->InitResult();
     if (init_res == LIBUSB_SUCCESS) {
-        adapter_scan_thread =
+        adapter_scan_thread = std::thread(&Adapter::AdapterScanThread, this);
-            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);
     }
@@ -90,15 +32,17 @@ Adapter::~Adapter() {
     Reset();
 }
 
-void Adapter::AdapterInputThread(std::stop_token stop_token) {
+void Adapter::AdapterInputThread() {
     LOG_DEBUG(Input, "GC Adapter input thread started");
     s32 payload_size{};
     AdapterPayload adapter_payload{};
 
-    adapter_scan_thread = {};
+    if (adapter_scan_thread.joinable()) {
+        adapter_scan_thread.join();
+    }
 
-    while (!stop_token.stop_requested()) {
+    while (adapter_input_thread_running) {
-        libusb_interrupt_transfer(usb_adapter_handle->get(), input_endpoint, adapter_payload.data(),
+        libusb_interrupt_transfer(usb_adapter_handle, input_endpoint, adapter_payload.data(),
                                   static_cast<s32>(adapter_payload.size()), &payload_size, 16);
         if (IsPayloadCorrect(adapter_payload, payload_size)) {
             UpdateControllers(adapter_payload);
@@ -108,8 +52,7 @@ void Adapter::AdapterInputThread(std::stop_token stop_token) {
     }
 
     if (restart_scan_thread) {
-        adapter_scan_thread =
+        adapter_scan_thread = std::thread(&Adapter::AdapterScanThread, this);
-            std::jthread([this](std::stop_token token) { AdapterScanThread(token); });
         restart_scan_thread = false;
     }
 }
@@ -121,7 +64,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.request_stop();
+            adapter_input_thread_running = false;
             restart_scan_thread = true;
         }
         return false;
@@ -153,7 +96,7 @@ void Adapter::UpdatePadType(std::size_t port, ControllerTypes pad_type) {
         return;
     }
     // Device changed reset device and set new type
-    pads[port] = {};
+    ResetDevice(port);
     pads[port].type = pad_type;
 }
 
@@ -270,8 +213,7 @@ void Adapter::SendVibrations() {
     const u8 p3 = pads[2].enable_vibration;
     const u8 p4 = pads[3].enable_vibration;
     std::array<u8, 5> payload = {rumble_command, p1, p2, p3, p4};
-    const int err =
+    const int err = libusb_interrupt_transfer(usb_adapter_handle, output_endpoint, payload.data(),
-        libusb_interrupt_transfer(usb_adapter_handle->get(), output_endpoint, payload.data(),
                                               static_cast<s32>(payload.size()), &size, 16);
     if (err) {
         LOG_DEBUG(Input, "Adapter libusb write failed: {}", libusb_error_name(err));
@@ -291,53 +233,56 @@ bool Adapter::RumblePlay(std::size_t port, u8 amplitude) {
     return rumble_enabled;
 }
 
-void Adapter::AdapterScanThread(std::stop_token stop_token) {
+void Adapter::AdapterScanThread() {
-    usb_adapter_handle = nullptr;
+    adapter_scan_thread_running = true;
-    pads = {};
+    adapter_input_thread_running = false;
-    while (!stop_token.stop_requested() && !Setup()) {
+    if (adapter_input_thread.joinable()) {
-        std::this_thread::sleep_for(std::chrono::seconds(2));
+        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));
     }
 }
 
-bool Adapter::Setup() {
+void Adapter::Setup() {
-    constexpr u16 nintendo_vid = 0x057e;
+    usb_adapter_handle = libusb_open_device_with_vid_pid(libusb_ctx, 0x057e, 0x0337);
-    constexpr u16 gc_adapter_pid = 0x0337;
+
-    usb_adapter_handle =
+    if (usb_adapter_handle == NULL) {
-        std::make_unique<LibUSBDeviceHandle>(libusb_ctx->get(), nintendo_vid, gc_adapter_pid);
+        return;
-    if (!usb_adapter_handle->get()) {
-        return false;
     }
     if (!CheckDeviceAccess()) {
-        usb_adapter_handle = nullptr;
+        ClearLibusbHandle();
-        return false;
+        return;
     }
 
-    libusb_device* const device = libusb_get_device(usb_adapter_handle->get());
+    libusb_device* device = libusb_get_device(usb_adapter_handle);
 
     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 =
+        adapter_input_thread = std::thread(&Adapter::AdapterInputThread, this);
-            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->get(), 0x21, 11, 0x0001, 0, nullptr, 0, 1000);
+        libusb_control_transfer(usb_adapter_handle, 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->get(), 0);
+    s32 kernel_driver_error = libusb_kernel_driver_active(usb_adapter_handle, 0);
     if (kernel_driver_error == 1) {
-        kernel_driver_error = libusb_detach_kernel_driver(usb_adapter_handle->get(), 0);
+        kernel_driver_error = libusb_detach_kernel_driver(usb_adapter_handle, 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);
@@ -345,13 +290,15 @@ 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->get(), 0);
+    const int interface_claim_error = libusb_claim_interface(usb_adapter_handle, 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;
     }
@@ -385,17 +332,57 @@ 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->get(), output_endpoint, &clear_payload,
+    libusb_interrupt_transfer(usb_adapter_handle, output_endpoint, &clear_payload,
                               sizeof(clear_payload), nullptr, 16);
     return true;
 }
 
-void Adapter::Reset() {
+void Adapter::JoinThreads() {
-    adapter_scan_thread = {};
+    restart_scan_thread = false;
-    adapter_input_thread = {};
+    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;
-    pads = {};
+    }
-    libusb_ctx = 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);
+    }
 }
 
 std::vector<Common::ParamPackage> Adapter::GetInputDevices() const {

src/input_common/gcadapter/gc_adapter.h

@@ -3,14 +3,11 @@
 // Refer to the license.txt file included.
 
 #pragma once
-
 #include <algorithm>
 #include <functional>
 #include <mutex>
-#include <stop_token>
 #include <thread>
 #include <unordered_map>
-
 #include "common/common_types.h"
 #include "common/threadsafe_queue.h"
 #include "input_common/main.h"
@@ -21,9 +18,6 @@ struct libusb_device_handle;
 
 namespace GCAdapter {
 
-class LibUSBContext;
-class LibUSBDeviceHandle;
-
 enum class PadButton {
     Undefined = 0x0000,
     ButtonLeft = 0x0001,
@@ -69,11 +63,11 @@ struct GCPadStatus {
 };
 
 struct GCController {
-    ControllerTypes type = ControllerTypes::None;
+    ControllerTypes type{};
-    bool enable_vibration = false;
+    bool enable_vibration{};
-    u8 rumble_amplitude = 0;
+    u8 rumble_amplitude{};
-    u16 buttons = 0;
+    u16 buttons{};
-    PadButton last_button = PadButton::Undefined;
+    PadButton last_button{};
     std::array<s16, 6> axis_values{};
     std::array<u8, 6> axis_origin{};
     u8 reset_origin_counter{};
@@ -115,9 +109,9 @@ private:
     void UpdateStateAxes(std::size_t port, const AdapterPayload& adapter_payload);
     void UpdateVibrations();
 
-    void AdapterInputThread(std::stop_token stop_token);
+    void AdapterInputThread();
 
-    void AdapterScanThread(std::stop_token stop_token);
+    void AdapterScanThread();
 
     bool IsPayloadCorrect(const AdapterPayload& adapter_payload, s32 payload_size);
 
@@ -125,7 +119,13 @@ private:
     void SendVibrations();
 
     /// For use in initialization, querying devices to find the adapter
-    bool Setup();
+    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);
 
     /// Returns true if we successfully gain access to GC Adapter
     bool CheckDeviceAccess();
@@ -137,15 +137,23 @@ private:
     /// For shutting down, clear all data, join all threads, release usb
     void Reset();
 
-    std::unique_ptr<LibUSBDeviceHandle> usb_adapter_handle;
+    // Join all threads
+    void JoinThreads();
+
+    // Release usb handles
+    void ClearLibusbHandle();
+
+    libusb_device_handle* usb_adapter_handle = nullptr;
     std::array<GCController, 4> pads;
     Common::SPSCQueue<GCPadStatus> pad_queue;
 
-    std::jthread adapter_input_thread;
+    std::thread adapter_input_thread;
-    std::jthread adapter_scan_thread;
+    std::thread adapter_scan_thread;
-    bool restart_scan_thread{};
+    bool adapter_input_thread_running;
+    bool adapter_scan_thread_running;
+    bool restart_scan_thread;
 
-    std::unique_ptr<LibUSBContext> libusb_ctx;
+    libusb_context* libusb_ctx;
 
     u8 input_endpoint{0};
     u8 output_endpoint{0};