Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use separate libusb contexts to avoid thread safety issues #8218

Merged
merged 2 commits into from Jul 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 4 additions & 8 deletions Source/Core/Core/IOS/USB/Bluetooth/BTReal.cpp
Expand Up @@ -28,7 +28,6 @@
#include "Core/Core.h"
#include "Core/HW/Memmap.h"
#include "Core/IOS/Device.h"
#include "Core/LibusbUtils.h"
#include "VideoCommon/OnScreenDisplay.h"

namespace IOS::HLE::Device
Expand Down Expand Up @@ -75,11 +74,10 @@ BluetoothReal::~BluetoothReal()

IPCCommandResult BluetoothReal::Open(const OpenRequest& request)
{
auto& context = LibusbUtils::GetContext();
if (!context.IsValid())
if (!m_context.IsValid())
return GetDefaultReply(IPC_EACCES);

context.GetDeviceList([this](libusb_device* device) {
m_context.GetDeviceList([this](libusb_device* device) {
libusb_device_descriptor device_descriptor;
libusb_get_device_descriptor(device, &device_descriptor);
auto config_descriptor = LibusbUtils::MakeConfigDescriptor(device);
Expand Down Expand Up @@ -594,8 +592,7 @@ void BluetoothReal::HandleCtrlTransfer(libusb_transfer* tr)
}
const auto& command = m_current_transfers.at(tr).command;
command->FillBuffer(libusb_control_transfer_get_data(tr), tr->actual_length);
m_ios.EnqueueIPCReply(command->ios_request, tr->actual_length, 0,
CoreTiming::FromThread::NON_CPU);
m_ios.EnqueueIPCReply(command->ios_request, tr->actual_length, 0, CoreTiming::FromThread::ANY);
m_current_transfers.erase(tr);
}

Expand Down Expand Up @@ -643,8 +640,7 @@ void BluetoothReal::HandleBulkOrIntrTransfer(libusb_transfer* tr)

const auto& command = m_current_transfers.at(tr).command;
command->FillBuffer(tr->buffer, tr->actual_length);
m_ios.EnqueueIPCReply(command->ios_request, tr->actual_length, 0,
CoreTiming::FromThread::NON_CPU);
m_ios.EnqueueIPCReply(command->ios_request, tr->actual_length, 0, CoreTiming::FromThread::ANY);
m_current_transfers.erase(tr);
}
} // namespace IOS::HLE::Device
2 changes: 2 additions & 0 deletions Source/Core/Core/IOS/USB/Bluetooth/BTReal.h
Expand Up @@ -19,6 +19,7 @@
#include "Core/IOS/USB/Bluetooth/BTBase.h"
#include "Core/IOS/USB/Bluetooth/hci.h"
#include "Core/IOS/USB/USBV0.h"
#include "Core/LibusbUtils.h"

class PointerWrap;
struct libusb_device;
Expand Down Expand Up @@ -70,6 +71,7 @@ class BluetoothReal final : public BluetoothBase
std::atomic<SyncButtonState> m_sync_button_state{SyncButtonState::Unpressed};
Common::Timer m_sync_button_held_timer;

LibusbUtils::Context m_context;
libusb_device* m_device = nullptr;
libusb_device_handle* m_handle = nullptr;

Expand Down
6 changes: 2 additions & 4 deletions Source/Core/Core/IOS/USB/Host.cpp
Expand Up @@ -24,7 +24,6 @@
#include "Core/Core.h"
#include "Core/IOS/USB/Common.h"
#include "Core/IOS/USB/LibusbDevice.h"
#include "Core/LibusbUtils.h"

namespace IOS::HLE::Device
{
Expand Down Expand Up @@ -121,10 +120,9 @@ bool USBHost::AddNewDevices(std::set<u64>& new_devices, DeviceChangeHooks& hooks
if (SConfig::GetInstance().m_usb_passthrough_devices.empty())
return true;

auto& context = LibusbUtils::GetContext();
if (context.IsValid())
if (m_context.IsValid())
{
context.GetDeviceList([&](libusb_device* device) {
m_context.GetDeviceList([&](libusb_device* device) {
libusb_device_descriptor descriptor;
libusb_get_device_descriptor(device, &descriptor);
const std::pair<u16, u16> vid_pid = {descriptor.idVendor, descriptor.idProduct};
Expand Down
2 changes: 2 additions & 0 deletions Source/Core/Core/IOS/USB/Host.h
Expand Up @@ -20,6 +20,7 @@
#include "Core/IOS/Device.h"
#include "Core/IOS/IOS.h"
#include "Core/IOS/USB/Common.h"
#include "Core/LibusbUtils.h"

class PointerWrap;

Expand Down Expand Up @@ -71,5 +72,6 @@ class USBHost : public Device
std::thread m_scan_thread;
Common::Event m_first_scan_complete_event;
bool m_has_initialised = false;
LibusbUtils::Context m_context;
};
} // namespace IOS::HLE::Device
6 changes: 0 additions & 6 deletions Source/Core/Core/LibusbUtils.cpp
Expand Up @@ -109,12 +109,6 @@ bool Context::GetDeviceList(GetDeviceListCallback 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)
{
#if defined(__LIBUSB__)
Expand Down
5 changes: 0 additions & 5 deletions Source/Core/Core/LibusbUtils.h
Expand Up @@ -38,11 +38,6 @@ class Context
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>;
ConfigDescriptor MakeConfigDescriptor(libusb_device* device, u8 config_num = 0);
} // namespace LibusbUtils
13 changes: 7 additions & 6 deletions Source/Core/InputCommon/GCAdapter.cpp
Expand Up @@ -72,6 +72,8 @@ static bool s_libusb_hotplug_enabled = false;
static libusb_hotplug_callback_handle s_hotplug_handle;
#endif

static LibusbUtils::Context s_libusb_context;

static u8 s_endpoint_in = 0;
static u8 s_endpoint_out = 0;

Expand Down Expand Up @@ -147,7 +149,6 @@ static int HotplugCallback(libusb_context* ctx, libusb_device* dev, libusb_hotpl

static void ScanThreadFunc()
{
auto& context = LibusbUtils::GetContext();
Common::SetCurrentThreadName("GC Adapter Scanning Thread");
NOTICE_LOG(SERIALINTERFACE, "GC Adapter scanning thread started");

Expand All @@ -158,7 +159,7 @@ static void ScanThreadFunc()
if (s_libusb_hotplug_enabled)
{
if (libusb_hotplug_register_callback(
context,
s_libusb_context,
(libusb_hotplug_event)(LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED |
LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT),
LIBUSB_HOTPLUG_ENUMERATE, 0x057e, 0x0337, LIBUSB_HOTPLUG_MATCH_ANY, HotplugCallback,
Expand Down Expand Up @@ -212,7 +213,7 @@ void StartScanThread()
{
if (s_adapter_detect_thread_running.IsSet())
return;
if (!LibusbUtils::GetContext().IsValid())
if (!s_libusb_context.IsValid())
return;
s_adapter_detect_thread_running.Set(true);
s_adapter_detect_thread = std::thread(ScanThreadFunc);
Expand Down Expand Up @@ -240,7 +241,7 @@ static void Setup()
s_controller_rumble[i] = 0;
}

LibusbUtils::GetContext().GetDeviceList([](libusb_device* device) {
s_libusb_context.GetDeviceList([](libusb_device* device) {
if (CheckDeviceAccess(device))
{
// Only connect to a single adapter in case the user has multiple connected
Expand Down Expand Up @@ -364,8 +365,8 @@ void Shutdown()
{
StopScanThread();
#if defined(LIBUSB_API_VERSION) && LIBUSB_API_VERSION >= 0x01000102
if (LibusbUtils::GetContext().IsValid() && s_libusb_hotplug_enabled)
libusb_hotplug_deregister_callback(LibusbUtils::GetContext(), s_hotplug_handle);
if (s_libusb_context.IsValid() && s_libusb_hotplug_enabled)
libusb_hotplug_deregister_callback(s_libusb_context, s_hotplug_handle);
#endif
Reset();

Expand Down
2 changes: 1 addition & 1 deletion Source/Core/UICommon/USBUtils.cpp
Expand Up @@ -35,7 +35,7 @@ std::map<std::pair<u16, u16>, std::string> GetInsertedDevices()
std::map<std::pair<u16, u16>, std::string> devices;

#ifdef __LIBUSB__
auto& context = LibusbUtils::GetContext();
LibusbUtils::Context context;
if (!context.IsValid())
return devices;

Expand Down