IOS: Fix savestates for Bluetooth passthrough

This fixes savestates when using Bluetooth passthrough by keeping track
of pending transfer commands and discarding them on state load, so that
the emulated software receives a reply to IOS requests as expected.

With this change, savestates in BT passthrough should work as long as
no Wiimote is connected when creating the savestate and when
restoring it. Yes, I know this is an important limitation -- but
that is probably the best we can do, and it's still better than
completely broken savestates.

Source/Core/Core/IOS/USB/Bluetooth/BTReal.cpp

@@ -7,10 +7,7 @@
 #include <cstring>
 #include <iomanip>
 #include <iterator>
-#include <map>
-#include <memory>
 #include <sstream>
-#include <string>
 #include <utility>
 #include <vector>
 
@@ -30,6 +27,7 @@
 #include "Core/IOS/Device.h"
 #include "Core/IOS/USB/Bluetooth/BTReal.h"
 #include "Core/IOS/USB/Bluetooth/hci.h"
+#include "VideoCommon/OnScreenDisplay.h"
 
 namespace IOS
 {
@@ -180,6 +178,7 @@ IPCCommandResult BluetoothReal::IOCtlV(const IOCtlVRequest& request)
   // HCI commands to the Bluetooth adapter
   case USB::IOCTLV_USBV0_CTRLMSG:
   {
+    std::lock_guard<std::mutex> lk(m_transfers_mutex);
     auto cmd = std::make_unique<USB::V0CtrlMessage>(request);
     const u16 opcode = Common::swap16(Memory::Read_U16(cmd->data_address));
     if (opcode == HCI_CMD_READ_BUFFER_SIZE)
@@ -215,8 +214,12 @@ IPCCommandResult BluetoothReal::IOCtlV(const IOCtlVRequest& request)
     Memory::CopyFromEmu(buffer.get() + LIBUSB_CONTROL_SETUP_SIZE, cmd->data_address, cmd->length);
     libusb_transfer* transfer = libusb_alloc_transfer(0);
     transfer->flags |= LIBUSB_TRANSFER_FREE_TRANSFER;
-    libusb_fill_control_transfer(transfer, m_handle, buffer.release(), CommandCallback,
+    libusb_fill_control_transfer(transfer, m_handle, buffer.get(), nullptr, this, 0);
-                                 cmd.release(), 0);
+    transfer->callback = [](libusb_transfer* tr) {
+      static_cast<BluetoothReal*>(tr->user_data)->HandleCtrlTransfer(tr);
+    };
+    PendingTransfer pending_transfer{std::move(cmd), std::move(buffer)};
+    m_current_transfers.emplace(transfer, std::move(pending_transfer));
     libusb_submit_transfer(transfer);
     break;
   }
@@ -224,43 +227,49 @@ IPCCommandResult BluetoothReal::IOCtlV(const IOCtlVRequest& request)
   case USB::IOCTLV_USBV0_BLKMSG:
   case USB::IOCTLV_USBV0_INTRMSG:
   {
-    auto buffer = std::make_unique<USB::V0IntrMessage>(request);
+    std::lock_guard<std::mutex> lk(m_transfers_mutex);
+    auto cmd = std::make_unique<USB::V0IntrMessage>(request);
     if (request.request == USB::IOCTLV_USBV0_INTRMSG)
     {
       if (m_sync_button_state == SyncButtonState::Pressed)
       {
         Core::DisplayMessage("Scanning for Wii Remotes", 2000);
-        FakeSyncButtonPressedEvent(*buffer);
+        FakeSyncButtonPressedEvent(*cmd);
         return GetNoReply();
       }
       if (m_sync_button_state == SyncButtonState::LongPressed)
       {
         Core::DisplayMessage("Reset saved Wii Remote pairings", 2000);
-        FakeSyncButtonHeldEvent(*buffer);
+        FakeSyncButtonHeldEvent(*cmd);
         return GetNoReply();
       }
       if (m_fake_read_buffer_size_reply.TestAndClear())
       {
-        FakeReadBufferSizeReply(*buffer);
+        FakeReadBufferSizeReply(*cmd);
         return GetNoReply();
       }
       if (m_fake_vendor_command_reply.TestAndClear())
       {
-        FakeVendorCommandReply(*buffer);
+        FakeVendorCommandReply(*cmd);
         return GetNoReply();
       }
     }
+    auto buffer = cmd->MakeBuffer(cmd->length);
     libusb_transfer* transfer = libusb_alloc_transfer(0);
-    transfer->buffer = Memory::GetPointer(buffer->data_address);
+    transfer->buffer = buffer.get();
-    transfer->callback = TransferCallback;
+    transfer->callback = [](libusb_transfer* tr) {
+      static_cast<BluetoothReal*>(tr->user_data)->HandleBulkOrIntrTransfer(tr);
+    };
     transfer->dev_handle = m_handle;
-    transfer->endpoint = buffer->endpoint;
+    transfer->endpoint = cmd->endpoint;
     transfer->flags |= LIBUSB_TRANSFER_FREE_TRANSFER;
-    transfer->length = buffer->length;
+    transfer->length = cmd->length;
     transfer->timeout = TIMEOUT;
     transfer->type = request.request == USB::IOCTLV_USBV0_BLKMSG ? LIBUSB_TRANSFER_TYPE_BULK :
                                                                    LIBUSB_TRANSFER_TYPE_INTERRUPT;
-    transfer->user_data = buffer.release();
+    transfer->user_data = this;
+    PendingTransfer pending_transfer{std::move(cmd), std::move(buffer)};
+    m_current_transfers.emplace(transfer, std::move(pending_transfer));
     libusb_submit_transfer(transfer);
     break;
   }
@@ -269,18 +278,60 @@ IPCCommandResult BluetoothReal::IOCtlV(const IOCtlVRequest& request)
   return GetNoReply();
 }
 
+static bool s_has_shown_savestate_warning = false;
 void BluetoothReal::DoState(PointerWrap& p)
 {
   bool passthrough_bluetooth = true;
   p.Do(passthrough_bluetooth);
-  if (p.GetMode() == PointerWrap::MODE_READ)
-    PanicAlertT("Attempted to load a state. Bluetooth will likely be broken now.");
-
   if (!passthrough_bluetooth && p.GetMode() == PointerWrap::MODE_READ)
   {
     Core::DisplayMessage("State needs Bluetooth passthrough to be disabled. Aborting load.", 4000);
     p.SetMode(PointerWrap::MODE_VERIFY);
+    return;
   }
+
+  // Prevent the transfer callbacks from messing with m_current_transfers after we have started
+  // writing a savestate. We cannot use a scoped lock here because DoState is called twice and
+  // we would lose the lock between the two calls.
+  if (p.GetMode() == PointerWrap::MODE_MEASURE || p.GetMode() == PointerWrap::MODE_VERIFY)
+    m_transfers_mutex.lock();
+
+  std::vector<u32> addresses_to_discard;
+  if (p.GetMode() != PointerWrap::MODE_READ)
+  {
+    // Save addresses of transfer commands to discard on savestate load.
+    for (const auto& transfer : m_current_transfers)
+      addresses_to_discard.push_back(transfer.second.command->ios_request.address);
+  }
+  p.Do(addresses_to_discard);
+  if (p.GetMode() == PointerWrap::MODE_READ)
+  {
+    // On load, discard any pending transfer to make sure the emulated software is not stuck
+    // waiting for the previous request to complete. This is usually not an issue as long as
+    // the Bluetooth state is the same (same Wii remote connections).
+    for (const auto& address_to_discard : addresses_to_discard)
+      EnqueueReply(Request{address_to_discard}, 0);
+
+    // Prevent the callbacks from replying to a request that has already been discarded.
+    m_current_transfers.clear();
+
+    OSD::AddMessage("If the savestate does not load correctly, disconnect all Wii Remotes "
+                    "and reload it.",
+                    OSD::Duration::NORMAL);
+  }
+
+  if (!s_has_shown_savestate_warning && p.GetMode() == PointerWrap::MODE_WRITE)
+  {
+    OSD::AddMessage("Savestates may not work with Bluetooth passthrough in all cases.\n"
+                    "They will only work if no remote is connected when restoring the state,\n"
+                    "or no remote is disconnected after saving.",
+                    OSD::Duration::VERY_LONG);
+    s_has_shown_savestate_warning = true;
+  }
+
+  // We have finished the savestate now, so the transfers mutex can be unlocked.
+  if (p.GetMode() == PointerWrap::MODE_WRITE)
+    m_transfers_mutex.unlock();
 }
 
 void BluetoothReal::UpdateSyncButtonState(const bool is_held)
@@ -565,10 +616,12 @@ void BluetoothReal::TransferThread()
 }
 
 // The callbacks are called from libusb code on a separate thread.
-void BluetoothReal::CommandCallback(libusb_transfer* tr)
+void BluetoothReal::HandleCtrlTransfer(libusb_transfer* tr)
 {
-  const std::unique_ptr<USB::CtrlMessage> cmd(static_cast<USB::CtrlMessage*>(tr->user_data));
+  std::lock_guard<std::mutex> lk(m_transfers_mutex);
-  const std::unique_ptr<u8[]> buffer(tr->buffer);
+  if (!m_current_transfers.count(tr))
+    return;
+
   if (tr->status != LIBUSB_TRANSFER_COMPLETED && tr->status != LIBUSB_TRANSFER_NO_DEVICE)
   {
     ERROR_LOG(IOS_WIIMOTE, "libusb command transfer failed, status: 0x%02x", tr->status);
@@ -583,13 +636,18 @@ void BluetoothReal::CommandCallback(libusb_transfer* tr)
   {
     s_showed_failed_transfer.Clear();
   }
-  cmd->FillBuffer(libusb_control_transfer_get_data(tr), tr->actual_length);
+  const auto& command = m_current_transfers.at(tr).command;
-  EnqueueReply(cmd->ios_request, tr->actual_length, 0, CoreTiming::FromThread::NON_CPU);
+  command->FillBuffer(libusb_control_transfer_get_data(tr), tr->actual_length);
+  EnqueueReply(command->ios_request, tr->actual_length, 0, CoreTiming::FromThread::NON_CPU);
+  m_current_transfers.erase(tr);
 }
 
-void BluetoothReal::TransferCallback(libusb_transfer* tr)
+void BluetoothReal::HandleBulkOrIntrTransfer(libusb_transfer* tr)
 {
-  const std::unique_ptr<USB::V0IntrMessage> ctrl(static_cast<USB::V0IntrMessage*>(tr->user_data));
+  std::lock_guard<std::mutex> lk(m_transfers_mutex);
+  if (!m_current_transfers.count(tr))
+    return;
+
   if (tr->status != LIBUSB_TRANSFER_COMPLETED && tr->status != LIBUSB_TRANSFER_TIMED_OUT &&
       tr->status != LIBUSB_TRANSFER_NO_DEVICE)
   {
@@ -627,7 +685,11 @@ void BluetoothReal::TransferCallback(libusb_transfer* tr)
       s_need_reset_keys.Set();
     }
   }
-  EnqueueReply(ctrl->ios_request, tr->actual_length, 0, CoreTiming::FromThread::NON_CPU);
+
+  const auto& command = m_current_transfers.at(tr).command;
+  command->FillBuffer(tr->buffer, tr->actual_length);
+  EnqueueReply(command->ios_request, tr->actual_length, 0, CoreTiming::FromThread::NON_CPU);
+  m_current_transfers.erase(tr);
 }
 }  // namespace Device
 }  // namespace HLE

Source/Core/Core/IOS/USB/Bluetooth/BTReal.h

@@ -7,6 +7,9 @@
 #if defined(__LIBUSB__)
 #include <array>
 #include <atomic>
+#include <map>
+#include <memory>
+#include <mutex>
 #include <string>
 #include <thread>
 
@@ -57,6 +60,9 @@ public:
   void TriggerSyncButtonPressedEvent() override;
   void TriggerSyncButtonHeldEvent() override;
 
+  void HandleCtrlTransfer(libusb_transfer* finished_transfer);
+  void HandleBulkOrIntrTransfer(libusb_transfer* finished_transfer);
+
 private:
   static constexpr u8 INTERFACE = 0x00;
   // Arbitrarily chosen value that allows emulated software to send commands often enough
@@ -75,6 +81,18 @@ private:
   Common::Flag m_thread_running;
   std::thread m_thread;
 
+  std::mutex m_transfers_mutex;
+  struct PendingTransfer
+  {
+    PendingTransfer(std::unique_ptr<USB::TransferCommand> command_, std::unique_ptr<u8[]> buffer_)
+        : command(std::move(command_)), buffer(std::move(buffer_))
+    {
+    }
+    std::unique_ptr<USB::TransferCommand> command;
+    std::unique_ptr<u8[]> buffer;
+  };
+  std::map<libusb_transfer*, PendingTransfer> m_current_transfers;
+
   // Set when we received a command to which we need to fake a reply
   Common::Flag m_fake_read_buffer_size_reply;
   Common::Flag m_fake_vendor_command_reply;
@@ -99,8 +117,6 @@ private:
   void StartTransferThread();
   void StopTransferThread();
   void TransferThread();
-  static void CommandCallback(libusb_transfer* transfer);
-  static void TransferCallback(libusb_transfer* transfer);
 };
 }  // namespace Device
 }  // namespace HLE

Source/Core/Core/State.cpp

@@ -71,7 +71,7 @@ static Common::Event g_compressAndDumpStateSyncEvent;
 static std::thread g_save_thread;
 
 // Don't forget to increase this after doing changes on the savestate system
-static const u32 STATE_VERSION = 72;  // Last changed in PR 4710
+static const u32 STATE_VERSION = 73;  // Last changed in PR 4651
 
 // Maps savestate versions to Dolphin versions.
 // Versions after 42 don't need to be added to this list,