Merge 6fa5a764b6 into 53b54406bd

Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/NativeLibrary.java

@@ -252,19 +252,15 @@ public final class NativeLibrary
    * Saves a game state to the slot number.
    *
    * @param slot The slot location to save state to.
-   * @param wait If false, returns as early as possible.
-   *             If true, returns once the savestate has been written to disk.
    */
-  public static native void SaveState(int slot, boolean wait);
+  public static native void SaveState(int slot);
 
   /**
    * Saves a game state to the specified path.
    *
    * @param path The path to save state to.
-   * @param wait If false, returns as early as possible.
-   *             If true, returns once the savestate has been written to disk.
    */
-  public static native void SaveStateAs(String path, boolean wait);
+  public static native void SaveStateAs(String path);
 
   /**
    * Loads a game state from the slot number.

Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/activities/EmulationActivity.kt

@@ -464,16 +464,16 @@ class EmulationActivity : AppCompatActivity(), ThemeProvider {
             }
 
             MENU_ACTION_TAKE_SCREENSHOT -> NativeLibrary.SaveScreenShot()
-            MENU_ACTION_QUICK_SAVE -> NativeLibrary.SaveState(9, false)
+            MENU_ACTION_QUICK_SAVE -> NativeLibrary.SaveState(9)
             MENU_ACTION_QUICK_LOAD -> NativeLibrary.LoadState(9)
             MENU_ACTION_SAVE_ROOT -> showSubMenu(SaveOrLoad.SAVE)
             MENU_ACTION_LOAD_ROOT -> showSubMenu(SaveOrLoad.LOAD)
-            MENU_ACTION_SAVE_SLOT1 -> NativeLibrary.SaveState(0, false)
-            MENU_ACTION_SAVE_SLOT2 -> NativeLibrary.SaveState(1, false)
-            MENU_ACTION_SAVE_SLOT3 -> NativeLibrary.SaveState(2, false)
-            MENU_ACTION_SAVE_SLOT4 -> NativeLibrary.SaveState(3, false)
-            MENU_ACTION_SAVE_SLOT5 -> NativeLibrary.SaveState(4, false)
-            MENU_ACTION_SAVE_SLOT6 -> NativeLibrary.SaveState(5, false)
+            MENU_ACTION_SAVE_SLOT1 -> NativeLibrary.SaveState(0)
+            MENU_ACTION_SAVE_SLOT2 -> NativeLibrary.SaveState(1)
+            MENU_ACTION_SAVE_SLOT3 -> NativeLibrary.SaveState(2)
+            MENU_ACTION_SAVE_SLOT4 -> NativeLibrary.SaveState(3)
+            MENU_ACTION_SAVE_SLOT5 -> NativeLibrary.SaveState(4)
+            MENU_ACTION_SAVE_SLOT6 -> NativeLibrary.SaveState(5)
             MENU_ACTION_LOAD_SLOT1 -> NativeLibrary.LoadState(0)
             MENU_ACTION_LOAD_SLOT2 -> NativeLibrary.LoadState(1)
             MENU_ACTION_LOAD_SLOT3 -> NativeLibrary.LoadState(2)

Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/EmulationFragment.kt

@@ -228,7 +228,7 @@ class EmulationFragment : Fragment(), SurfaceHolder.Callback {
         }
     }
 
-    fun saveTemporaryState() = NativeLibrary.SaveStateAs(temporaryStateFilePath, true)
+    fun saveTemporaryState() = NativeLibrary.SaveStateAs(temporaryStateFilePath)
 
     private val temporaryStateFilePath: String
         get() = "${requireContext().filesDir}${File.separator}temp.sav"

Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/utils/ActivityTracker.kt

@@ -56,10 +56,21 @@ class ActivityTracker : ActivityLifecycleCallbacks {
 
     override fun onActivitySaveInstanceState(activity: Activity, bundle: Bundle) {}
 
+    override fun onActivityPostSaveInstanceState(activity: Activity, bundle: Bundle) {
+        if (DirectoryInitialization.areDolphinDirectoriesReady() &&
+            !activity.isChangingConfigurations
+        ) {
+            flushUnsavedData()
+        }
+    }
+
     override fun onActivityDestroyed(activity: Activity) {}
 
     companion object {
         @JvmStatic
         external fun setBackgroundExecutionAllowedNative(allowed: Boolean)
+
+        @JvmStatic
+        external fun flushUnsavedData()
     }
 }

Source/Android/jni/ActivityTracker.cpp

@@ -5,6 +5,8 @@
 
 #include "Common/Logging/Log.h"
 #include "Core/AchievementManager.h"
+#include "UICommon/UICommon.h"
+#include "jni/Host.h"
 
 extern "C" {
 
@@ -18,4 +20,11 @@ Java_org_dolphinemu_dolphinemu_utils_ActivityTracker_setBackgroundExecutionAllow
   INFO_LOG_FMT(CORE, "SetBackgroundExecutionAllowed {}", allowed);
   AchievementManager::GetInstance().SetBackgroundExecutionAllowed(allowed);
 }
+
+JNIEXPORT void JNICALL
+Java_org_dolphinemu_dolphinemu_utils_ActivityTracker_flushUnsavedData(JNIEnv*, jclass)
+{
+  HostThreadLock guard;
+  UICommon::FlushUnsavedData();
+}
 }

Source/Android/jni/MainAndroid.cpp

@@ -334,19 +334,17 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_eglBindAPI(J
 }
 
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SaveState(JNIEnv*, jclass,
-                                                                              jint slot,
-                                                                              jboolean wait)
+                                                                              jint slot)
 {
   HostThreadLock guard;
-  State::Save(Core::System::GetInstance(), slot, wait);
+  State::Save(Core::System::GetInstance(), slot);
 }
 
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SaveStateAs(JNIEnv* env, jclass,
-                                                                                jstring path,
-                                                                                jboolean wait)
+                                                                                jstring path)
 {
   HostThreadLock guard;
-  State::SaveAs(Core::System::GetInstance(), GetJString(env, path), wait);
+  State::SaveAs(Core::System::GetInstance(), GetJString(env, path));
 }
 
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_LoadState(JNIEnv*, jclass,

Source/Core/Common/Thread.h

@@ -3,6 +3,7 @@
 
 #pragma once
 
+#include <atomic>
 #include <thread>
 
 #ifndef _WIN32

Source/Core/Common/WaitableFlag.h

@@ -66,4 +66,42 @@ private:
   std::mutex m_mutex;
 };
 
+// A thread-safe counter that can be waited on.
+// std::mutex interface is provided for use with scope-based locks.
+// lock/unlock just change the internal counter. They alone don't provide thread synchronization.
+// Add functionality as needed.
+template <typename T>
+class WaitableCounter
+{
+public:
+  explicit WaitableCounter(T initial_value = {}) : m_counter{initial_value} {}
+
+  void lock(T count = 1)
+  {
+    m_counter.fetch_add(count, std::memory_order_release);
+    m_counter.notify_all();
+  }
+  void unlock(T count = 1) { lock(0 - count); }
+
+  // Waits until lock'ed an exact amount of times.
+  void WaitForValue(const T& value)
+  {
+    while (true)
+    {
+      const auto current_value = m_counter.load(std::memory_order_acquire);
+      if (current_value == value)
+        break;
+      WaitForValueChange(current_value);
+    }
+  }
+
+  void WaitForValueChange(const T& old_value)
+  {
+    m_counter.wait(old_value, std::memory_order_acquire);
+  }
+
+private:
+  std::atomic<T> m_counter{};
+};
+
 }  // namespace Common

Source/Core/Core/Core.h

@@ -16,6 +16,7 @@
 
 #include "Common/CommonTypes.h"
 #include "Common/Functional.h"
+#include "Common/HookableEvent.h"
 
 struct BootParameters;
 struct WindowSystemInfo;
@@ -195,4 +196,7 @@ void UpdateInputGate(bool require_focus, bool require_full_focus = false);
 
 void UpdateTitle(Core::System& system);
 
+// Triggered from the Host-thread on Android before a potential process termination.
+using FlushUnsavedDataEvent = Common::HookableEvent<"FlushUnsavedData">;
+
 }  // namespace Core

Source/Core/Core/State.cpp

@@ -4,11 +4,9 @@
 #include "Core/State.h"
 
 #include <algorithm>
-#include <condition_variable>
 #include <filesystem>
 #include <locale>
 #include <map>
-#include <memory>
 #include <mutex>
 #include <string>
 #include <utility>
@@ -20,16 +18,18 @@
 #include <lz4.h>
 #include <lzo/lzo1x.h>
 
+#include "Common/Buffer.h"
 #include "Common/ChunkFile.h"
 #include "Common/CommonTypes.h"
 #include "Common/Contains.h"
-#include "Common/Event.h"
 #include "Common/FileUtil.h"
 #include "Common/IOFile.h"
+#include "Common/Logging/Log.h"
 #include "Common/MsgHandler.h"
 #include "Common/Thread.h"
 #include "Common/TimeUtil.h"
 #include "Common/Version.h"
+#include "Common/WaitableFlag.h"
 #include "Common/WorkQueueThread.h"
 
 #include "Core/AchievementManager.h"
@@ -68,31 +68,27 @@ static AfterLoadCallbackFunc s_on_after_load_callback;
 
 // Temporary undo state buffer
 static Common::UniqueBuffer<u8> s_undo_load_buffer;
-static std::mutex s_undo_load_buffer_mutex;
 
-static std::mutex s_load_or_save_in_progress_mutex;
+// Used to estimate buffer size for the next save.
+static u32 s_last_state_size = 0;
 
-struct CompressAndDumpState_args
+// Counter is incremented in state save calls that will write to the filesystem.
+// It won't again reach zero until the task is done writing to the filesystem.
+static Common::WaitableCounter<u32> s_state_saves_in_progress;
+
+// Protect concurrent access of states on the filesystem.
+// Note: Only the worker thread writes to these files.
+static std::mutex s_state_fs_mutex;
+
+struct CompressAndDumpStateArgs
 {
   Common::UniqueBuffer<u8> buffer;
   std::string filename;
-  std::shared_ptr<Common::Event> state_write_done_event;
+  std::unique_lock<decltype(s_state_saves_in_progress)> write_lock;
 };
 
-// Protects against simultaneous reads and writes to the final savestate location from multiple
-// threads.
-static std::mutex s_save_thread_mutex;
-
 // Queue for compressing and writing savestates to disk.
-static Common::WorkQueueThread<CompressAndDumpState_args> s_save_thread;
-
-// Keeps track of savestate writes that are currently happening, so we don't load a state while
-// another one is still saving. This is particularly important so if you save to a slot and then
-// immediately load from the same one, you don't accidentally load the state that's still at that
-// file path before the write is done.
-static std::mutex s_state_writes_in_queue_mutex;
-static size_t s_state_writes_in_queue;
-static std::condition_variable s_state_write_queue_is_empty;
+static Common::WorkQueueThreadSP<CompressAndDumpStateArgs> s_state_write_thread;
 
 // Don't forget to increase this after doing changes on the savestate system
 constexpr u32 STATE_VERSION = 174;  // Last changed in PR 13342
@@ -121,20 +117,31 @@ static const std::map<u32, std::pair<std::string, std::string>> s_old_versions =
     {38, {"4.0-4963", "4.0-5267"}}, {39, {"4.0-5279", "4.0-5525"}}, {40, {"4.0-5531", "4.0-5809"}},
     {41, {"4.0-5811", "4.0-5923"}}, {42, {"4.0-5925", "4.0-5946"}}};
 
-enum
-{
-  STATE_NONE = 0,
-  STATE_SAVE = 1,
-  STATE_LOAD = 2,
-};
+static constexpr bool s_use_compression = true;
 
-static bool s_use_compression = true;
-
-void EnableCompression(bool compression)
+// Acquired when starting a task that will write state saves to the filesystem.
+// It's not a real mutex, it can be moved to other threads.
+[[nodiscard]] static auto GetStateSaveTaskLock()
 {
-  s_use_compression = compression;
+  return std::unique_lock{s_state_saves_in_progress};
 }
 
+// Waits for previously requested state saves to reach the filesystem.
+static void WaitForStateSaveTaskCompletion()
+{
+  // Note: Because CPU-thread might be in a task itself, we instead wait for the worker.
+  // Being the CPU-thread, we would have already queued previous work.
+  if (Core::IsCPUThread())
+    s_state_write_thread.WaitForCompletion();
+  else
+    s_state_saves_in_progress.WaitForValue(0);
+}
+
+static Common::EventHook s_flush_data_hook =
+    Core::FlushUnsavedDataEvent::Register(&WaitForStateSaveTaskCompletion, "State flush");
+
+static bool ReadHeader(const std::string& filename, StateHeader& header);
+
 static void DoState(Core::System& system, PointerWrap& p)
 {
   bool is_wii = system.IsWii() || system.IsMIOS();
@@ -201,68 +208,81 @@ static void DoState(Core::System& system, PointerWrap& p)
 #endif  // USE_RETRO_ACHIEVEMENTS
 }
 
-void LoadFromBuffer(Core::System& system, Common::UniqueBuffer<u8>& buffer)
+static bool CheckIfStateLoadIsAllowed(Core::System& system)
 {
+  if (!Core::IsRunningOrStarting(system))
+    return false;
+
   if (NetPlay::IsNetPlayRunning())
   {
     OSD::AddMessage("Loading savestates is disabled in Netplay to prevent desyncs");
-    return;
+    return false;
   }
 
   if (AchievementManager::GetInstance().IsHardcoreModeActive())
   {
     OSD::AddMessage("Loading savestates is disabled in RetroAchievements hardcore mode");
-    return;
+    return false;
   }
 
-  Core::RunOnCPUThread(
-      system,
-      [&] {
-        u8* ptr = buffer.data();
-        PointerWrap p(&ptr, buffer.size(), PointerWrap::Mode::Read);
-        DoState(system, p);
-      },
-      true);
+  return true;
 }
 
-void SaveToBuffer(Core::System& system, Common::UniqueBuffer<u8>& buffer)
+static bool LoadFromBuffer(Core::System& system, std::span<u8> buffer)
 {
-  Core::RunOnCPUThread(
-      system,
-      [&] {
-        u8* ptr = nullptr;
-        PointerWrap p_measure(&ptr, 0, PointerWrap::Mode::Measure);
-        DoState(system, p_measure);
+  u8* ptr = buffer.data();
+  PointerWrap p(&ptr, buffer.size(), PointerWrap::Mode::Read);
+  DoState(system, p);
+  return p.IsReadMode();
+}
 
-        const size_t new_buffer_size = ptr - (u8*)(nullptr);
-        if (new_buffer_size > buffer.size())
-          buffer.reset(new_buffer_size);
+static bool SaveToBuffer(Core::System& system, Common::UniqueBuffer<u8>& buffer)
+{
+  // Attempt to save to our provided buffer as-is.
+  // If buffer isn't large enough, PointerWrap transitions to MeasureMode,
+  //  and then we have our measurement for a second attempt.
+  u8* ptr = buffer.data();
+  PointerWrap pointer_wrap(&ptr, buffer.size(), PointerWrap::Mode::Write);
+  DoState(system, pointer_wrap);
+  const auto measured_size = pointer_wrap.GetOffsetFromPreviousPosition(buffer.data());
 
-        ptr = buffer.data();
-        PointerWrap p(&ptr, buffer.size(), PointerWrap::Mode::Write);
-        DoState(system, p);
-      },
-      true);
+  if (pointer_wrap.IsWriteMode())
+  {
+    s_last_state_size = measured_size;
+    return true;
+  }
+
+  if (measured_size > buffer.size())
+  {
+    DEBUG_LOG_FMT(CORE, "SaveToBuffer: Growing buffer from size {} to measured size {}",
+                  buffer.size(), measured_size);
+    buffer.reset(measured_size);
+    return SaveToBuffer(system, buffer);
+  }
+
+  // Buffer was large enough but we still failed for some other reason.
+  return false;
 }
 
 namespace
 {
 struct SlotWithTimestamp
 {
+  // 1-based indexing.
   int slot;
   double timestamp;
 };
 }  // namespace
 
-// returns first slot number not in the vector, or -1 if all are in the vector
-static int GetEmptySlot(const std::vector<SlotWithTimestamp>& used_slots)
+// Returns first slot number (1-based indexing) not in the vector.
+static std::optional<int> GetEmptySlot(const std::vector<SlotWithTimestamp>& used_slots)
 {
-  for (int i = 1; i <= (int)NUM_STATES; i++)
+  for (int i = 1; i <= int(NUM_STATES); ++i)
   {
     if (!Common::Contains(used_slots, i, &SlotWithTimestamp::slot))
       return i;
   }
-  return -1;
+  return std::nullopt;
 }
 
 // Arbitrarily chosen value (38 years) that is subtracted in GetSystemTimeAsDouble()
@@ -289,40 +309,41 @@ static std::string SystemTimeAsDoubleToString(double time)
   return fmt::format(std::locale{""}, "{:%x %X}", *local_time);
 }
 
-static std::string MakeStateFilename(int number);
+static std::string MakeStateFilename(int number)
+{
+  return fmt::format("{}{}.s{:02d}", File::GetUserPath(D_STATESAVES_IDX),
+                     SConfig::GetInstance().GetGameID(), number);
+}
 
 static std::vector<SlotWithTimestamp> GetUsedSlotsWithTimestamp()
 {
   std::vector<SlotWithTimestamp> result;
   StateHeader header;
-  for (int i = 1; i <= (int)NUM_STATES; i++)
+  for (int i = 1; i <= int(NUM_STATES); ++i)
   {
     std::string filename = MakeStateFilename(i);
-    if (File::Exists(filename))
-    {
-      if (ReadHeader(filename, header))
-      {
-        result.emplace_back(SlotWithTimestamp{.slot = i, .timestamp = header.legacy_header.time});
-      }
-    }
+    if (!File::Exists(filename) || !ReadHeader(filename, header))
+      continue;
+
+    result.emplace_back(SlotWithTimestamp{.slot = i, .timestamp = header.legacy_header.time});
   }
   return result;
 }
 
-static void CompressBufferToFile(const u8* raw_buffer, u64 size, File::IOFile& f)
+static void CompressBufferToFile(std::span<const u8> raw_buffer, File::IOFile& f)
 {
   u64 total_bytes_compressed = 0;
 
   while (true)
   {
-    const u64 bytes_left_to_compress = size - total_bytes_compressed;
+    const u64 bytes_left_to_compress = raw_buffer.size() - total_bytes_compressed;
 
     const int bytes_to_compress =
         static_cast<int>(std::min(static_cast<u64>(LZ4_MAX_INPUT_SIZE), bytes_left_to_compress));
     Common::UniqueBuffer<char> compressed_buffer(LZ4_compressBound(bytes_to_compress));
     const int compressed_len = LZ4_compress_default(
-        reinterpret_cast<const char*>(raw_buffer) + total_bytes_compressed, compressed_buffer.get(),
-        bytes_to_compress, int(compressed_buffer.size()));
+        reinterpret_cast<const char*>(raw_buffer.data()) + total_bytes_compressed,
+        compressed_buffer.get(), bytes_to_compress, int(compressed_buffer.size()));
 
     if (compressed_len == 0)
     {
@@ -335,7 +356,7 @@ static void CompressBufferToFile(const u8* raw_buffer, u64 size, File::IOFile& f
     f.WriteBytes(compressed_buffer.get(), compressed_len);
 
     total_bytes_compressed += bytes_to_compress;
-    if (total_bytes_compressed == size)
+    if (total_bytes_compressed == raw_buffer.size())
       break;
   }
 }
@@ -374,17 +395,18 @@ static void WriteHeadersToFile(size_t uncompressed_size, File::IOFile& f)
   // If StateExtendedHeader is amended to include more than the base, add WriteBytes() calls here.
 }
 
-static void CompressAndDumpState(Core::System& system, CompressAndDumpState_args& save_args)
+static void CompressAndDumpState(Core::System& system, const CompressAndDumpStateArgs& save_args)
 {
   const u8* const buffer_data = save_args.buffer.data();
   const size_t buffer_size = save_args.buffer.size();
   const std::string& filename = save_args.filename;
 
   // Find free temporary filename.
-  // TODO: The file exists check and the actual opening of the file should be atomic, we don't have
-  // functions for that.
+
+  // TODO: The file exists check and the actual opening of the file should be atomic.
+  // This is only an issue for multiple instances of dolphin operating on the same user folder.
   std::string temp_filename;
-  size_t temp_counter = static_cast<size_t>(Common::CurrentThreadId());
+  auto temp_counter = static_cast<size_t>(Common::CurrentThreadId());
   do
   {
     temp_filename = fmt::format("{}{}.tmp", filename, temp_counter);
@@ -401,7 +423,7 @@ static void CompressAndDumpState(Core::System& system, CompressAndDumpState_args
   WriteHeadersToFile(buffer_size, f);
 
   if (s_use_compression)
-    CompressBufferToFile(buffer_data, buffer_size, f);
+    CompressBufferToFile(save_args.buffer, f);
   else
     f.WriteBytes(buffer_data, buffer_size);
 
@@ -413,7 +435,7 @@ static void CompressAndDumpState(Core::System& system, CompressAndDumpState_args
   const std::string dtmname = filename + ".dtm";
 
   {
-    std::lock_guard lk(s_save_thread_mutex);
+    std::lock_guard lk{s_state_fs_mutex};
 
     // Backup existing state (overwriting an existing backup, if any).
     if (File::Exists(filename))
@@ -460,65 +482,37 @@ static void CompressAndDumpState(Core::System& system, CompressAndDumpState_args
   Host_UpdateMainFrame();
 }
 
-void SaveAs(Core::System& system, const std::string& filename, bool wait)
+static void SaveAsFromCore(Core::System& system, std::string filename)
 {
-  std::unique_lock lk(s_load_or_save_in_progress_mutex, std::try_to_lock);
-  if (!lk)
-    return;
+  // Try with a buffer a bit larger than the previous state.
+  // This will often avoid the "Measure" step.
+  const auto buffer_size_estimate = std::size_t(s_last_state_size) * 110 / 100;
+  Common::UniqueBuffer<u8> buffer{buffer_size_estimate};
 
+  if (SaveToBuffer(system, buffer))
+  {
+    CompressAndDumpStateArgs dump_args;
+    dump_args.buffer = std::move(buffer);
+    dump_args.filename = std::move(filename);
+    dump_args.write_lock = GetStateSaveTaskLock();
+
+    Core::DisplayMessage("Saving State...", 1000);
+    s_state_write_thread.EmplaceItem(std::move(dump_args));
+  }
+  else
+  {
+    Core::DisplayMessage("Unable to save: Internal DoState Error", 4000);
+  }
+}
+
+void SaveAs(Core::System& system, std::string filename)
+{
   Core::RunOnCPUThread(
       system,
-      [&] {
-        {
-          std::lock_guard lk_(s_state_writes_in_queue_mutex);
-          ++s_state_writes_in_queue;
-        }
-
-        // Measure the size of the buffer.
-        u8* ptr = nullptr;
-        PointerWrap p_measure(&ptr, 0, PointerWrap::Mode::Measure);
-        DoState(system, p_measure);
-        const size_t buffer_size = ptr - (u8*)(nullptr);
-
-        // Then actually do the write.
-        Common::UniqueBuffer<u8> current_buffer(buffer_size);
-        ptr = current_buffer.data();
-        PointerWrap p(&ptr, buffer_size, PointerWrap::Mode::Write);
-        DoState(system, p);
-
-        if (p.IsWriteMode())
-        {
-          Core::DisplayMessage("Saving State...", 1000);
-
-          std::shared_ptr<Common::Event> sync_event;
-
-          CompressAndDumpState_args save_args;
-          save_args.buffer = std::move(current_buffer);
-          save_args.filename = filename;
-          if (wait)
-          {
-            sync_event = std::make_shared<Common::Event>();
-            save_args.state_write_done_event = sync_event;
-          }
-
-          s_save_thread.EmplaceItem(std::move(save_args));
-
-          if (sync_event)
-            sync_event->Wait();
-        }
-        else
-        {
-          // someone aborted the save by changing the mode?
-          {
-            // Note: The worker thread takes care of this in the other branch.
-            std::lock_guard lk_(s_state_writes_in_queue_mutex);
-            if (--s_state_writes_in_queue == 0)
-              s_state_write_queue_is_empty.notify_all();
-          }
-          Core::DisplayMessage("Unable to save: Internal DoState Error", 4000);
-        }
+      [&system, filename = std::move(filename), lock = GetStateSaveTaskLock()] {
+        SaveAsFromCore(system, filename);
       },
-      true);
+      false);
 }
 
 static bool GetVersionFromLZO(StateHeader& header, File::IOFile& f)
@@ -618,18 +612,18 @@ static bool ReadStateHeaderFromFile(StateHeader& header, File::IOFile& f,
   return true;
 }
 
-bool ReadHeader(const std::string& filename, StateHeader& header)
+static bool ReadHeader(const std::string& filename, StateHeader& header)
 {
-  // ensure that the savestate write thread isn't moving around states while we do this
-  std::lock_guard lk(s_save_thread_mutex);
-
   File::IOFile f(filename, "rb");
-  bool get_version_header = false;
+  constexpr bool get_version_header = false;
   return ReadStateHeaderFromFile(header, f, get_version_header);
 }
 
 std::string GetInfoStringOfSlot(int slot, bool translate)
 {
+  WaitForStateSaveTaskCompletion();
+  std::lock_guard lk{s_state_fs_mutex};
+
   std::string filename = MakeStateFilename(slot);
   if (!File::Exists(filename))
     return translate ? Common::GetStringT("Empty") : "Empty";
@@ -643,6 +637,9 @@ std::string GetInfoStringOfSlot(int slot, bool translate)
 
 u64 GetUnixTimeOfSlot(int slot)
 {
+  WaitForStateSaveTaskCompletion();
+  std::lock_guard lk{s_state_fs_mutex};
+
   State::StateHeader header;
   if (!ReadHeader(MakeStateFilename(slot), header))
     return 0;
@@ -659,7 +656,7 @@ static bool DecompressLZ4(Common::UniqueBuffer<u8>& raw_buffer, u64 size, File::
   u64 total_bytes_read = 0;
   while (true)
   {
-    s32 compressed_data_len;
+    s32 compressed_data_len = 0;
     if (!f.ReadArray(&compressed_data_len, 1))
     {
       PanicAlertFmt("Could not read state data length");
@@ -679,8 +676,8 @@ static bool DecompressLZ4(Common::UniqueBuffer<u8>& raw_buffer, u64 size, File::
       return false;
     }
 
-    u32 max_decompress_size =
-        static_cast<u32>(std::min((u64)LZ4_MAX_INPUT_SIZE, size - total_bytes_read));
+    const auto max_decompress_size =
+        static_cast<int>(std::min((u64)LZ4_MAX_INPUT_SIZE, size - total_bytes_read));
 
     int bytes_read = LZ4_decompress_safe(
         compressed_data.get(), reinterpret_cast<char*>(raw_buffer.data()) + total_bytes_read,
@@ -699,7 +696,8 @@ static bool DecompressLZ4(Common::UniqueBuffer<u8>& raw_buffer, u64 size, File::
     {
       return true;
     }
-    else if (total_bytes_read > size)
+
+    if (total_bytes_read > size)
     {
       PanicAlertFmtT("Internal LZ4 Error - payload size mismatch ({0} / {1}))", total_bytes_read,
                      size);
@@ -713,7 +711,7 @@ static bool ValidateHeaders(const StateHeader& header)
   bool success = true;
 
   // Game ID
-  if (strncmp(SConfig::GetInstance().GetGameID().c_str(), header.legacy_header.game_id, 6))
+  if (!std::ranges::equal(SConfig::GetInstance().GetGameID(), header.legacy_header.game_id))
   {
     Core::DisplayMessage(fmt::format("State belongs to a different game (ID {})",
                                      std::string_view{header.legacy_header.game_id,
@@ -722,8 +720,8 @@ static bool ValidateHeaders(const StateHeader& header)
     return false;
   }
 
-  // Check both the state version and the revision string
-  std::string current_str = Common::GetScmRevStr();
+  // Check the state version.
+  // FYI: We don't require an exact revision string match.
   std::string loaded_str = header.version_string;
   const u32 loaded_version = header.version_header.version_cookie - COOKIE_BASE;
 
@@ -758,22 +756,7 @@ static bool ValidateHeaders(const StateHeader& header)
 static void LoadFileStateData(const std::string& filename, Common::UniqueBuffer<u8>& ret_data)
 {
   File::IOFile f;
-
-  {
-    // If a state is currently saving, wait for that to end or time out.
-    std::unique_lock lk(s_state_writes_in_queue_mutex);
-    if (s_state_writes_in_queue != 0)
-    {
-      if (!s_state_write_queue_is_empty.wait_for(lk, std::chrono::seconds(3),
-                                                 []() { return s_state_writes_in_queue == 0; }))
-      {
-        Core::DisplayMessage(
-            "A previous state saving operation is still in progress, cancelling load.", 2000);
-        return;
-      }
-    }
-    f.Open(filename, "rb");
-  }
+  f.Open(filename, "rb");
 
   StateHeader header;
   if (!ReadStateHeaderFromFile(header, f) || !ValidateHeaders(header))
@@ -837,87 +820,74 @@ static void LoadFileStateData(const std::string& filename, Common::UniqueBuffer<
   ret_data.swap(buffer);
 }
 
-void LoadAs(Core::System& system, const std::string& filename)
+static void LoadAsFromCore(Core::System& system, std::string filename)
 {
-  if (!Core::IsRunningOrStarting(system))
-    return;
-
-  if (NetPlay::IsNetPlayRunning())
+  // Save temp buffer for undo load state
+  auto& movie = system.GetMovie();
+  if (!movie.IsJustStartingRecordingInputFromSaveState())
   {
-    OSD::AddMessage("Loading savestates is disabled in Netplay to prevent desyncs");
-    return;
+    SaveToBuffer(system, s_undo_load_buffer);
+    const std::string dtmpath = File::GetUserPath(D_STATESAVES_IDX) + "undo.dtm";
+    if (movie.IsMovieActive())
+      movie.SaveRecording(dtmpath);
+    else if (File::Exists(dtmpath))
+      File::Delete(dtmpath);
   }
 
-  if (AchievementManager::GetInstance().IsHardcoreModeActive())
+  bool was_file_read = false;
+  bool loaded_successfully = false;
+
+  // brackets here are so buffer gets freed ASAP
   {
-    OSD::AddMessage("Loading savestates is disabled in RetroAchievements hardcore mode");
-    return;
+    WaitForStateSaveTaskCompletion();
+
+    Common::UniqueBuffer<u8> buffer;
+    LoadFileStateData(filename, buffer);
+
+    if (!buffer.empty())
+    {
+      was_file_read = true;
+      loaded_successfully = LoadFromBuffer(system, buffer);
+    }
   }
 
-  std::unique_lock lk(s_load_or_save_in_progress_mutex, std::try_to_lock);
-  if (!lk)
+  if (was_file_read)
+  {
+    if (loaded_successfully)
+    {
+      std::filesystem::path temp_filename(std::move(filename));
+      Core::DisplayMessage(fmt::format("Loaded State from {}", temp_filename.filename().string()),
+                           2000);
+      if (File::Exists(filename + ".dtm"))
+        movie.LoadInput(filename + ".dtm");
+      else if (!movie.IsJustStartingRecordingInputFromSaveState() &&
+               !movie.IsJustStartingPlayingInputFromSaveState())
+        movie.EndPlayInput(false);
+    }
+    else
+    {
+      Core::DisplayMessage("The savestate could not be loaded", OSD::Duration::NORMAL);
+
+      // since we could be in an inconsistent state now (and might crash or whatever), undo.
+      UndoLoadState(system);
+    }
+  }
+
+  if (s_on_after_load_callback)
+    s_on_after_load_callback();
+}
+
+void LoadAs(Core::System& system, std::string filename)
+{
+  if (!CheckIfStateLoadIsAllowed(system))
     return;
 
   Core::RunOnCPUThread(
       system,
-      [&] {
-        // Save temp buffer for undo load state
-        auto& movie = system.GetMovie();
-        if (!movie.IsJustStartingRecordingInputFromSaveState())
-        {
-          std::lock_guard lk2(s_undo_load_buffer_mutex);
-          SaveToBuffer(system, s_undo_load_buffer);
-          const std::string dtmpath = File::GetUserPath(D_STATESAVES_IDX) + "undo.dtm";
-          if (movie.IsMovieActive())
-            movie.SaveRecording(dtmpath);
-          else if (File::Exists(dtmpath))
-            File::Delete(dtmpath);
-        }
-
-        bool loaded = false;
-        bool loadedSuccessfully = false;
-
-        // brackets here are so buffer gets freed ASAP
-        {
-          Common::UniqueBuffer<u8> buffer;
-          LoadFileStateData(filename, buffer);
-
-          if (!buffer.empty())
-          {
-            u8* ptr = buffer.data();
-            PointerWrap p(&ptr, buffer.size(), PointerWrap::Mode::Read);
-            DoState(system, p);
-            loaded = true;
-            loadedSuccessfully = p.IsReadMode();
-          }
-        }
-
-        if (loaded)
-        {
-          if (loadedSuccessfully)
-          {
-            std::filesystem::path tempfilename(filename);
-            Core::DisplayMessage(
-                fmt::format("Loaded State from {}", tempfilename.filename().string()), 2000);
-            if (File::Exists(filename + ".dtm"))
-              movie.LoadInput(filename + ".dtm");
-            else if (!movie.IsJustStartingRecordingInputFromSaveState() &&
-                     !movie.IsJustStartingPlayingInputFromSaveState())
-              movie.EndPlayInput(false);
-          }
-          else
-          {
-            Core::DisplayMessage("The savestate could not be loaded", OSD::Duration::NORMAL);
-
-            // since we could be in an inconsistent state now (and might crash or whatever), undo.
-            UndoLoadState(system);
-          }
-        }
-
-        if (s_on_after_load_callback)
-          s_on_after_load_callback();
+      [&system, filename = std::move(filename)]() mutable {
+        LoadAsFromCore(system, std::move(filename));
       },
-      true);
+      false);
 }
 
 void SetOnAfterLoadCallback(AfterLoadCallbackFunc callback)
@@ -927,37 +897,19 @@ void SetOnAfterLoadCallback(AfterLoadCallbackFunc callback)
 
 void Init(Core::System& system)
 {
-  s_save_thread.Reset("Savestate Worker", [&system](CompressAndDumpState_args args) {
-    CompressAndDumpState(system, args);
-
-    {
-      std::lock_guard lk(s_state_writes_in_queue_mutex);
-      if (--s_state_writes_in_queue == 0)
-        s_state_write_queue_is_empty.notify_all();
-    }
-
-    if (args.state_write_done_event)
-      args.state_write_done_event->Set();
-  });
+  s_state_write_thread.Reset("Savestate Worker",
+                             std::bind_front(&CompressAndDumpState, std::ref(system)));
 }
 
 void Shutdown()
 {
-  s_save_thread.Shutdown();
-
-  std::lock_guard lk(s_undo_load_buffer_mutex);
+  s_state_write_thread.Shutdown();
   s_undo_load_buffer.reset();
 }
 
-static std::string MakeStateFilename(int number)
+void Save(Core::System& system, int slot)
 {
-  return fmt::format("{}{}.s{:02d}", File::GetUserPath(D_STATESAVES_IDX),
-                     SConfig::GetInstance().GetGameID(), number);
-}
-
-void Save(Core::System& system, int slot, bool wait)
-{
-  SaveAs(system, MakeStateFilename(slot), wait);
+  SaveAs(system, MakeStateFilename(slot));
 }
 
 void Load(Core::System& system, int slot)
@@ -967,68 +919,85 @@ void Load(Core::System& system, int slot)
 
 void LoadLastSaved(Core::System& system, int i)
 {
-  if (i <= 0)
-  {
-    Core::DisplayMessage("State doesn't exist", 2000);
+  if (!CheckIfStateLoadIsAllowed(system))
     return;
-  }
 
-  std::vector<SlotWithTimestamp> used_slots = GetUsedSlotsWithTimestamp();
-  if (static_cast<size_t>(i) > used_slots.size())
-  {
-    Core::DisplayMessage("State doesn't exist", 2000);
-    return;
-  }
+  Core::RunOnCPUThread(
+      system,
+      [&system, i] {
+        WaitForStateSaveTaskCompletion();
 
-  std::ranges::stable_sort(used_slots, {}, &SlotWithTimestamp::timestamp);
-  Load(system, (used_slots.end() - i)->slot);
+        std::vector<SlotWithTimestamp> used_slots = GetUsedSlotsWithTimestamp();
+        if (std::size_t(i) > used_slots.size())
+        {
+          Core::DisplayMessage("State doesn't exist", 2000);
+          return;
+        }
+
+        std::ranges::stable_sort(used_slots, std::ranges::greater{}, &SlotWithTimestamp::timestamp);
+        LoadAsFromCore(system, MakeStateFilename(used_slots[i].slot));
+      },
+      false);
 }
 
-// must wait for state to be written because it must know if all slots are taken
 void SaveFirstSaved(Core::System& system)
 {
-  std::vector<SlotWithTimestamp> used_slots = GetUsedSlotsWithTimestamp();
-  if (used_slots.size() < NUM_STATES)
-  {
-    // save to an empty slot
-    Save(system, GetEmptySlot(used_slots), true);
-    return;
-  }
+  Core::RunOnCPUThread(
+      system,
+      [&system, lock = GetStateSaveTaskLock()] {
+        WaitForStateSaveTaskCompletion();
 
-  // overwrite the oldest state
-  std::ranges::stable_sort(used_slots, {}, &SlotWithTimestamp::timestamp);
-  Save(system, used_slots.front().slot, true);
+        std::vector<SlotWithTimestamp> used_slots = GetUsedSlotsWithTimestamp();
+        auto slot = GetEmptySlot(used_slots);
+        if (!slot.has_value())
+        {
+          // overwrite the oldest state
+          std::ranges::stable_sort(used_slots, {}, &SlotWithTimestamp::timestamp);
+          slot = used_slots.front().slot;
+        }
+
+        SaveAsFromCore(system, MakeStateFilename(*slot));
+      },
+      false);
 }
 
 // Load the last state before loading the state
 void UndoLoadState(Core::System& system)
 {
-  std::lock_guard lk(s_undo_load_buffer_mutex);
-  if (!s_undo_load_buffer.empty())
-  {
-    auto& movie = system.GetMovie();
-    if (movie.IsMovieActive())
-    {
-      const std::string dtmpath = File::GetUserPath(D_STATESAVES_IDX) + "undo.dtm";
-      if (File::Exists(dtmpath))
-      {
-        LoadFromBuffer(system, s_undo_load_buffer);
-        movie.LoadInput(dtmpath);
-      }
-      else
-      {
-        PanicAlertFmtT("No undo.dtm found, aborting undo load state to prevent movie desyncs");
-      }
-    }
-    else
-    {
-      LoadFromBuffer(system, s_undo_load_buffer);
-    }
-  }
-  else
-  {
-    PanicAlertFmtT("There is nothing to undo!");
-  }
+  if (!CheckIfStateLoadIsAllowed(system))
+    return;
+
+  Core::RunOnCPUThread(
+      system,
+      [&system] {
+        if (!s_undo_load_buffer.empty())
+        {
+          auto& movie = system.GetMovie();
+          if (movie.IsMovieActive())
+          {
+            const std::string dtmpath = File::GetUserPath(D_STATESAVES_IDX) + "undo.dtm";
+            if (File::Exists(dtmpath))
+            {
+              LoadFromBuffer(system, s_undo_load_buffer);
+              movie.LoadInput(dtmpath);
+            }
+            else
+            {
+              PanicAlertFmtT(
+                  "No undo.dtm found, aborting undo load state to prevent movie desyncs");
+            }
+          }
+          else
+          {
+            LoadFromBuffer(system, s_undo_load_buffer);
+          }
+        }
+        else
+        {
+          PanicAlertFmtT("There is nothing to undo!");
+        }
+      },
+      false);
 }
 
 // Load the state that the last save state overwritten on

Source/Core/Core/State.h

@@ -10,7 +10,6 @@
 #include <string>
 #include <type_traits>
 
-#include "Common/Buffer.h"
 #include "Common/CommonTypes.h"
 
 namespace Core
@@ -81,13 +80,8 @@ struct StateExtendedHeader
 };
 
 void Init(Core::System& system);
-
 void Shutdown();
 
-void EnableCompression(bool compression);
-
-bool ReadHeader(const std::string& filename, StateHeader& header);
-
 // Returns a string containing information of the savestate in the given slot
 // which can be presented to the user for identification purposes
 std::string GetInfoStringOfSlot(int slot, bool translate = true);
@@ -100,14 +94,11 @@ u64 GetUnixTimeOfSlot(int slot);
 //    If we're in the main CPU thread then they run immediately instead
 //    because some things (like Lua) need them to run immediately.
 // Slots from 0-99.
-void Save(Core::System& system, int slot, bool wait = false);
+void Save(Core::System& system, int slot);
 void Load(Core::System& system, int slot);
 
-void SaveAs(Core::System& system, const std::string& filename, bool wait = false);
-void LoadAs(Core::System& system, const std::string& filename);
-
-void SaveToBuffer(Core::System& system, Common::UniqueBuffer<u8>& buffer);
-void LoadFromBuffer(Core::System& system, const Common::UniqueBuffer<u8>& buffer);
+void SaveAs(Core::System& system, std::string filename);
+void LoadAs(Core::System& system, std::string filename);
 
 void LoadLastSaved(Core::System& system, int i = 1);
 void SaveFirstSaved(Core::System& system);

Source/Core/DolphinQt/MenuBar.cpp

@@ -170,7 +170,6 @@ void MenuBar::OnEmulationStateChanged(Core::State state)
   // Symbols
   m_symbols->setEnabled(running);
 
-  UpdateStateSlotMenu();
   UpdateToolsMenu(state);
 
   OnDebugModeToggled(Settings::Instance().IsDebugModeEnabled());

Source/Core/UICommon/UICommon.cpp

@@ -158,6 +158,12 @@ void Shutdown()
   Config::Shutdown();
 }
 
+void FlushUnsavedData()
+{
+  INFO_LOG_FMT(CORE, "Flushing unsaved data...");
+  Core::FlushUnsavedDataEvent::Trigger();
+}
+
 void InitControllers(const WindowSystemInfo& wsi)
 {
   if (g_controller_interface.IsInit())

Source/Core/UICommon/UICommon.h

@@ -14,6 +14,8 @@ namespace UICommon
 void Init();
 void Shutdown();
 
+void FlushUnsavedData();
+
 void InitControllers(const WindowSystemInfo& wsi);
 void ShutdownControllers();