Use separate libusb contexts to avoid thread safety issues

Unfortunately, it appears that using libusb's synchronous transfer API
from several threads causes nasty race conditions in event handling and
can lead to deadlocks, despite the fact that libusb's synchronous API
is documented to be perfectly fine to use from several threads (only
the manual polling functionality is supposed to require special
precautions).

Since usbdk was the only real reason for using a single libusb context
and since usbdk (currently) has so many issues with Dolphin, I think
dropping support for it in order to fix other backends is acceptable.
This commit is contained in:
Léo Lam
2019-06-25 17:31:35 +02:00
parent 769ba43abf
commit cf60a9a7f7
8 changed files with 16 additions and 26 deletions

View File

@ -28,7 +28,6 @@
#include "Core/Core.h" #include "Core/Core.h"
#include "Core/HW/Memmap.h" #include "Core/HW/Memmap.h"
#include "Core/IOS/Device.h" #include "Core/IOS/Device.h"
#include "Core/LibusbUtils.h"
#include "VideoCommon/OnScreenDisplay.h" #include "VideoCommon/OnScreenDisplay.h"
namespace IOS::HLE::Device namespace IOS::HLE::Device
@ -75,11 +74,10 @@ BluetoothReal::~BluetoothReal()
IPCCommandResult BluetoothReal::Open(const OpenRequest& request) IPCCommandResult BluetoothReal::Open(const OpenRequest& request)
{ {
auto& context = LibusbUtils::GetContext(); if (!m_context.IsValid())
if (!context.IsValid())
return GetDefaultReply(IPC_EACCES); return GetDefaultReply(IPC_EACCES);
context.GetDeviceList([this](libusb_device* device) { m_context.GetDeviceList([this](libusb_device* device) {
libusb_device_descriptor device_descriptor; libusb_device_descriptor device_descriptor;
libusb_get_device_descriptor(device, &device_descriptor); libusb_get_device_descriptor(device, &device_descriptor);
auto config_descriptor = LibusbUtils::MakeConfigDescriptor(device); auto config_descriptor = LibusbUtils::MakeConfigDescriptor(device);

View File

@ -19,6 +19,7 @@
#include "Core/IOS/USB/Bluetooth/BTBase.h" #include "Core/IOS/USB/Bluetooth/BTBase.h"
#include "Core/IOS/USB/Bluetooth/hci.h" #include "Core/IOS/USB/Bluetooth/hci.h"
#include "Core/IOS/USB/USBV0.h" #include "Core/IOS/USB/USBV0.h"
#include "Core/LibusbUtils.h"
class PointerWrap; class PointerWrap;
struct libusb_device; struct libusb_device;
@ -70,6 +71,7 @@ private:
std::atomic<SyncButtonState> m_sync_button_state{SyncButtonState::Unpressed}; std::atomic<SyncButtonState> m_sync_button_state{SyncButtonState::Unpressed};
Common::Timer m_sync_button_held_timer; Common::Timer m_sync_button_held_timer;
LibusbUtils::Context m_context;
libusb_device* m_device = nullptr; libusb_device* m_device = nullptr;
libusb_device_handle* m_handle = nullptr; libusb_device_handle* m_handle = nullptr;

View File

@ -24,7 +24,6 @@
#include "Core/Core.h" #include "Core/Core.h"
#include "Core/IOS/USB/Common.h" #include "Core/IOS/USB/Common.h"
#include "Core/IOS/USB/LibusbDevice.h" #include "Core/IOS/USB/LibusbDevice.h"
#include "Core/LibusbUtils.h"
namespace IOS::HLE::Device namespace IOS::HLE::Device
{ {
@ -121,10 +120,9 @@ bool USBHost::AddNewDevices(std::set<u64>& new_devices, DeviceChangeHooks& hooks
if (SConfig::GetInstance().m_usb_passthrough_devices.empty()) if (SConfig::GetInstance().m_usb_passthrough_devices.empty())
return true; return true;
auto& context = LibusbUtils::GetContext(); if (m_context.IsValid())
if (context.IsValid())
{ {
context.GetDeviceList([&](libusb_device* device) { m_context.GetDeviceList([&](libusb_device* device) {
libusb_device_descriptor descriptor; libusb_device_descriptor descriptor;
libusb_get_device_descriptor(device, &descriptor); libusb_get_device_descriptor(device, &descriptor);
const std::pair<u16, u16> vid_pid = {descriptor.idVendor, descriptor.idProduct}; const std::pair<u16, u16> vid_pid = {descriptor.idVendor, descriptor.idProduct};

View File

@ -20,6 +20,7 @@
#include "Core/IOS/Device.h" #include "Core/IOS/Device.h"
#include "Core/IOS/IOS.h" #include "Core/IOS/IOS.h"
#include "Core/IOS/USB/Common.h" #include "Core/IOS/USB/Common.h"
#include "Core/LibusbUtils.h"
class PointerWrap; class PointerWrap;
@ -71,5 +72,6 @@ private:
std::thread m_scan_thread; std::thread m_scan_thread;
Common::Event m_first_scan_complete_event; Common::Event m_first_scan_complete_event;
bool m_has_initialised = false; bool m_has_initialised = false;
LibusbUtils::Context m_context;
}; };
} // namespace IOS::HLE::Device } // namespace IOS::HLE::Device

View File

@ -109,12 +109,6 @@ bool Context::GetDeviceList(GetDeviceListCallback callback)
return m_impl->GetDeviceList(std::move(callback)); return m_impl->GetDeviceList(std::move(callback));
} }
Context& GetContext()
{
static Context s_context;
return s_context;
}
ConfigDescriptor MakeConfigDescriptor(libusb_device* device, u8 config_num) ConfigDescriptor MakeConfigDescriptor(libusb_device* device, u8 config_num)
{ {
#if defined(__LIBUSB__) #if defined(__LIBUSB__)

View File

@ -38,11 +38,6 @@ private:
std::unique_ptr<Impl> m_impl; std::unique_ptr<Impl> m_impl;
}; };
// Use this to get a libusb context. Do *not* use any other context
// because some libusb backends such as UsbDk only work properly with a single context.
// Additionally, device lists must be retrieved using GetDeviceList for thread safety reasons.
Context& GetContext();
using ConfigDescriptor = UniquePtr<libusb_config_descriptor>; using ConfigDescriptor = UniquePtr<libusb_config_descriptor>;
ConfigDescriptor MakeConfigDescriptor(libusb_device* device, u8 config_num = 0); ConfigDescriptor MakeConfigDescriptor(libusb_device* device, u8 config_num = 0);
} // namespace LibusbUtils } // namespace LibusbUtils

View File

@ -72,6 +72,8 @@ static bool s_libusb_hotplug_enabled = false;
static libusb_hotplug_callback_handle s_hotplug_handle; static libusb_hotplug_callback_handle s_hotplug_handle;
#endif #endif
static LibusbUtils::Context s_libusb_context;
static u8 s_endpoint_in = 0; static u8 s_endpoint_in = 0;
static u8 s_endpoint_out = 0; static u8 s_endpoint_out = 0;
@ -147,7 +149,6 @@ static int HotplugCallback(libusb_context* ctx, libusb_device* dev, libusb_hotpl
static void ScanThreadFunc() static void ScanThreadFunc()
{ {
auto& context = LibusbUtils::GetContext();
Common::SetCurrentThreadName("GC Adapter Scanning Thread"); Common::SetCurrentThreadName("GC Adapter Scanning Thread");
NOTICE_LOG(SERIALINTERFACE, "GC Adapter scanning thread started"); NOTICE_LOG(SERIALINTERFACE, "GC Adapter scanning thread started");
@ -158,7 +159,7 @@ static void ScanThreadFunc()
if (s_libusb_hotplug_enabled) if (s_libusb_hotplug_enabled)
{ {
if (libusb_hotplug_register_callback( if (libusb_hotplug_register_callback(
context, s_libusb_context,
(libusb_hotplug_event)(LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED | (libusb_hotplug_event)(LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED |
LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT), LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT),
LIBUSB_HOTPLUG_ENUMERATE, 0x057e, 0x0337, LIBUSB_HOTPLUG_MATCH_ANY, HotplugCallback, LIBUSB_HOTPLUG_ENUMERATE, 0x057e, 0x0337, LIBUSB_HOTPLUG_MATCH_ANY, HotplugCallback,
@ -212,7 +213,7 @@ void StartScanThread()
{ {
if (s_adapter_detect_thread_running.IsSet()) if (s_adapter_detect_thread_running.IsSet())
return; return;
if (!LibusbUtils::GetContext().IsValid()) if (!s_libusb_context.IsValid())
return; return;
s_adapter_detect_thread_running.Set(true); s_adapter_detect_thread_running.Set(true);
s_adapter_detect_thread = std::thread(ScanThreadFunc); s_adapter_detect_thread = std::thread(ScanThreadFunc);
@ -240,7 +241,7 @@ static void Setup()
s_controller_rumble[i] = 0; s_controller_rumble[i] = 0;
} }
LibusbUtils::GetContext().GetDeviceList([](libusb_device* device) { s_libusb_context.GetDeviceList([](libusb_device* device) {
if (CheckDeviceAccess(device)) if (CheckDeviceAccess(device))
{ {
// Only connect to a single adapter in case the user has multiple connected // Only connect to a single adapter in case the user has multiple connected
@ -364,8 +365,8 @@ void Shutdown()
{ {
StopScanThread(); StopScanThread();
#if defined(LIBUSB_API_VERSION) && LIBUSB_API_VERSION >= 0x01000102 #if defined(LIBUSB_API_VERSION) && LIBUSB_API_VERSION >= 0x01000102
if (LibusbUtils::GetContext().IsValid() && s_libusb_hotplug_enabled) if (s_libusb_context.IsValid() && s_libusb_hotplug_enabled)
libusb_hotplug_deregister_callback(LibusbUtils::GetContext(), s_hotplug_handle); libusb_hotplug_deregister_callback(s_libusb_context, s_hotplug_handle);
#endif #endif
Reset(); Reset();

View File

@ -35,7 +35,7 @@ std::map<std::pair<u16, u16>, std::string> GetInsertedDevices()
std::map<std::pair<u16, u16>, std::string> devices; std::map<std::pair<u16, u16>, std::string> devices;
#ifdef __LIBUSB__ #ifdef __LIBUSB__
auto& context = LibusbUtils::GetContext(); LibusbUtils::Context context;
if (!context.IsValid()) if (!context.IsValid())
return devices; return devices;