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

@@ -3,6 +3,7 @@
 #include <string>
 #include <switch.h>
 #include <vector>
+#include <mutex>
 
 #include "textures.hpp"
 
@@ -47,6 +48,7 @@ typedef struct {
 extern WindowData data;
 extern int sort;
 extern std::vector<std::string> devices_list;
+extern std::recursive_mutex devices_list_mutex;
 
 namespace FileBrowser {
     bool Sort(const FsDirectoryEntry &entryA, const FsDirectoryEntry &entryB);

source/tabs/filebrowser.cpp

@@ -11,6 +11,7 @@
 
 int sort = 0;
 std::vector<std::string> 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]);
 

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;
     }
 }