From 4e4624136a102cf43e7d4f14ffa3110433387a5e Mon Sep 17 00:00:00 2001 From: Pablo Curiel Date: Tue, 13 Sep 2022 21:58:16 +0200 Subject: [PATCH] usb: Lock devices_list access behind a recursive mutex (#125) Fixes devices list population after a UMS device has been mounted by libusbhsfs. Also tweaked USB namespace functions to better deal with thread/pointer safety. --- include/windows.hpp | 2 + source/tabs/filebrowser.cpp | 3 + source/usb.cpp | 120 +++++++++++++++++++++--------------- 3 files changed, 75 insertions(+), 50 deletions(-) diff --git a/include/windows.hpp b/include/windows.hpp index 4e520a5..0256000 100644 --- a/include/windows.hpp +++ b/include/windows.hpp @@ -3,6 +3,7 @@ #include #include #include +#include #include "textures.hpp" @@ -47,6 +48,7 @@ typedef struct { extern WindowData data; extern int sort; extern std::vector devices_list; +extern std::recursive_mutex devices_list_mutex; namespace FileBrowser { bool Sort(const FsDirectoryEntry &entryA, const FsDirectoryEntry &entryB); diff --git a/source/tabs/filebrowser.cpp b/source/tabs/filebrowser.cpp index ddaca7a..d51a32a 100644 --- a/source/tabs/filebrowser.cpp +++ b/source/tabs/filebrowser.cpp @@ -11,6 +11,7 @@ int sort = 0; std::vector devices_list = { "sdmc:", "safe:", "user:", "system:" }; +std::recursive_mutex devices_list_mutex; namespace FileBrowser { // Sort without using ImGuiTableSortSpecs @@ -87,6 +88,8 @@ namespace Tabs { ImGui::PushID("device_list"); ImGui::PushItemWidth(160.f); if (ImGui::BeginCombo("", device.c_str())) { + std::scoped_lock lock(devices_list_mutex); + for (std::size_t i = 0; i < devices_list.size(); i++) { const bool is_selected = (device == devices_list[i]); diff --git a/source/usb.cpp b/source/usb.cpp index 7bb80c1..fe5f3b8 100644 --- a/source/usb.cpp +++ b/source/usb.cpp @@ -8,8 +8,9 @@ namespace USB { static UEvent *status_change_event = nullptr, exit_event = {0}; static u32 usb_device_count = 0; static UsbHsFsDevice *usb_devices = nullptr; - static Thread thread; + static Thread thread = {0}; static u32 listed_device_count = 0; + static bool thread_created = false; // This function is heavily based off the example provided by DarkMatterCore // https://github.com/DarkMatterCore/libusbhsfs/blob/main/example/source/main.c @@ -31,32 +32,35 @@ namespace USB { /* Exit event triggered. */ if (idx == 1) break; - - /* Get mounted device count. */ - usb_device_count = usbHsFsGetMountedDeviceCount(); - - if (!usb_device_count) { - USB::Unmount(); - continue; - } - /* Free mounted devices buffer. */ - if (usb_devices) - delete[] usb_devices; - - /* Allocate mounted devices buffer. */ - usb_devices = new UsbHsFsDevice[usb_device_count]; - if (!usb_devices) - continue; - - /* List mounted devices. */ - if (!(listed_device_count = usbHsFsListMountedDevices(usb_devices, usb_device_count))) - continue; - - /* Print info from mounted devices. */ - for(u32 i = 0; i < listed_device_count; i++) { - UsbHsFsDevice *device = std::addressof(usb_devices[i]); - devices_list.push_back(device->name); + /* Free USB Mass Storage device data. */ + USB::Unmount(); + + { + std::scoped_lock lock(devices_list_mutex); + + /* Get mounted device count. */ + usb_device_count = usbHsFsGetMountedDeviceCount(); + if (!usb_device_count) + continue; + + /* Allocate mounted devices buffer. */ + usb_devices = new UsbHsFsDevice[usb_device_count]; + if (!usb_devices) + continue; + + /* List mounted devices. */ + if (!(listed_device_count = usbHsFsListMountedDevices(usb_devices, usb_device_count))) { + delete[] usb_devices; + usb_devices = nullptr; + continue; + } + + /* Print info from mounted devices. */ + for(u32 i = 0; i < listed_device_count; i++) { + UsbHsFsDevice *device = std::addressof(usb_devices[i]); + devices_list.push_back(device->name); + } } } @@ -65,49 +69,65 @@ namespace USB { } Result Init(void) { + std::scoped_lock lock(devices_list_mutex); + Result ret = usbHsFsInitialize(0); - - /* Get USB Mass Storage status change event. */ - status_change_event = usbHsFsGetStatusChangeUserEvent(); - - /* Create usermode thread exit event. */ - ueventCreate(&exit_event, true); - - /* Create thread. */ - if (R_SUCCEEDED(ret = threadCreate(&thread, usbMscThreadFunc, nullptr, nullptr, 0x10000, 0x2C, -2))) - ret = threadStart(&thread); - + if (R_SUCCEEDED(ret)) { + /* Get USB Mass Storage status change event. */ + status_change_event = usbHsFsGetStatusChangeUserEvent(); + + /* Create usermode thread exit event. */ + ueventCreate(&exit_event, true); + + /* Create thread. */ + if (R_SUCCEEDED(ret = threadCreate(&thread, usbMscThreadFunc, nullptr, nullptr, 0x10000, 0x2C, -2))) { + if (R_SUCCEEDED(ret = threadStart(&thread))) + thread_created = true; + } + } + return ret; } void Exit(void) { - /* Signal background thread. */ - ueventSignal(&exit_event); + std::scoped_lock lock(devices_list_mutex); + + if (thread_created) { + /* Signal background thread. */ + ueventSignal(&exit_event); + + /* Wait for the background thread to exit on its own. */ + threadWaitForExit(&thread); + threadClose(&thread); + + thread_created = false; + } - /* Wait for the background thread to exit on its own. */ - threadWaitForExit(&thread); - threadClose(&thread); - /* Clean up and exit. */ USB::Unmount(); usbHsFsExit(); } bool Connected(void) { + std::scoped_lock lock(devices_list_mutex); return (listed_device_count > 0); } void Unmount(void) { + std::scoped_lock lock(devices_list_mutex); + /* Unmount devices. */ - for(u32 i = 0; i < listed_device_count; i++) { - UsbHsFsDevice *device = std::addressof(usb_devices[i]); - devices_list.pop_back(); - usbHsFsUnmountDevice(device, false); + if (usb_devices) { + for(u32 i = 0; i < listed_device_count; i++) { + UsbHsFsDevice *device = std::addressof(usb_devices[i]); + devices_list.pop_back(); + usbHsFsUnmountDevice(device, false); + } + + delete[] usb_devices; + usb_devices = nullptr; } listed_device_count = 0; - - if (usb_devices) - delete[] usb_devices; } }