Merge pull request #13594 from jordan-woyak/state-cleanups

State: Simplify interthread communication and general cleanups.

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

@@ -233,19 +233,17 @@ object 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.
      */
     @JvmStatic
-    external fun SaveState(slot: Int, wait: Boolean)
+    external fun SaveState(slot: Int)
 
     /**
      * 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.
      */
     @JvmStatic
-    external fun SaveStateAs(path: String, wait: Boolean)
+    external fun SaveStateAs(path: String)
 
     /**
      * Loads a game state from the slot number.

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

@@ -494,16 +494,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

@@ -232,7 +232,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

@@ -310,19 +310,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/CMakeLists.txt

@@ -151,6 +151,7 @@ add_library(common
   Timer.h
   TimeUtil.cpp
   TimeUtil.h
+  TransferableSharedMutex.h
   TraversalClient.cpp
   TraversalClient.h
   TraversalProto.h

Source/Core/Common/TransferableSharedMutex.h

@@ -0,0 +1,92 @@
+// Copyright 2025 Dolphin Emulator Project
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#pragma once
+
+#include <atomic>
+#include <cassert>
+#include <cstdint>
+
+namespace Common
+{
+// Behaves like `std::shared_mutex` but locks and unlocks may come from different threads.
+class TransferableSharedMutex
+{
+public:
+  void lock()
+  {
+    while (true)
+    {
+      CounterType old_value{};
+      if (m_counter.compare_exchange_strong(old_value, EXCLUSIVE_LOCK_VALUE,
+                                            std::memory_order_acquire, std::memory_order_relaxed))
+      {
+        return;
+      }
+
+      // lock() or lock_shared() is already held.
+      // Wait for an unlock notification and try again.
+      m_counter.wait(old_value, std::memory_order_relaxed);
+    }
+  }
+
+  bool try_lock()
+  {
+    CounterType old_value{};
+    return m_counter.compare_exchange_weak(old_value, EXCLUSIVE_LOCK_VALUE,
+                                           std::memory_order_acquire, std::memory_order_relaxed);
+  }
+
+  void unlock()
+  {
+    m_counter.store(0, std::memory_order_release);
+    m_counter.notify_all();  // Notify potentially multiple wait()ers in lock_shared().
+  }
+
+  void lock_shared()
+  {
+    while (true)
+    {
+      auto old_value = m_counter.load(std::memory_order_relaxed);
+      while (old_value < LAST_SHARED_LOCK_VALUE)
+      {
+        if (m_counter.compare_exchange_strong(old_value, old_value + 1, std::memory_order_acquire,
+                                              std::memory_order_relaxed))
+        {
+          return;
+        }
+      }
+
+      // Something has gone very wrong if m_counter is nearly saturated with shared_lock().
+      assert(old_value != LAST_SHARED_LOCK_VALUE);
+
+      // lock() is already held.
+      // Wait for an unlock notification and try again.
+      m_counter.wait(old_value, std::memory_order_relaxed);
+    }
+  }
+
+  bool try_lock_shared()
+  {
+    auto old_value = m_counter.load(std::memory_order_relaxed);
+    return (old_value < LAST_SHARED_LOCK_VALUE) &&
+           m_counter.compare_exchange_weak(old_value, old_value + 1, std::memory_order_acquire,
+                                           std::memory_order_relaxed);
+  }
+
+  void unlock_shared()
+  {
+    if (m_counter.fetch_sub(1, std::memory_order_release) == 1)
+      m_counter.notify_one();  // Notify one of the wait()ers in lock().
+  }
+
+private:
+  using CounterType = std::uintptr_t;
+
+  static constexpr auto EXCLUSIVE_LOCK_VALUE = CounterType(-1);
+  static constexpr auto LAST_SHARED_LOCK_VALUE = EXCLUSIVE_LOCK_VALUE - 1;
+
+  std::atomic<CounterType> m_counter{};
+};
+
+}  // namespace Common

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);
@@ -97,17 +91,12 @@ u64 GetUnixTimeOfSlot(int slot);
 
 // These don't happen instantly - they get scheduled as events.
 // ...But only if we're not in the main CPU thread.
-//    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);
+//    If we're in the main CPU thread then they run immediately instead.
+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, 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/DolphinLib.props

@@ -171,6 +171,7 @@
     <ClInclude Include="Common\Thread.h" />
     <ClInclude Include="Common\Timer.h" />
     <ClInclude Include="Common\TimeUtil.h" />
+    <ClInclude Include="Common\TransferableSharedMutex.h" />
     <ClInclude Include="Common\TraversalClient.h" />
     <ClInclude Include="Common\TraversalProto.h" />
     <ClInclude Include="Common\TypeUtils.h" />

Source/Core/UICommon/UICommon.cpp

@@ -60,6 +60,7 @@
 namespace UICommon
 {
 static Config::ConfigChangedCallbackID s_config_changed_callback_id;
+static Common::HookableEvent<> s_flush_unsaved_data_event_hook;
 
 static void CreateDumpPath(std::string path)
 {
@@ -157,6 +158,17 @@ void Shutdown()
   Config::Shutdown();
 }
 
+[[nodiscard]] Common::EventHook AddFlushUnsavedDataCallback(std::function<void()> callback)
+{
+  return s_flush_unsaved_data_event_hook.Register(std::move(callback));
+}
+
+void FlushUnsavedData()
+{
+  INFO_LOG_FMT(CORE, "Flushing unsaved data...");
+  s_flush_unsaved_data_event_hook.Trigger();
+}
+
 void InitControllers(const WindowSystemInfo& wsi)
 {
   if (g_controller_interface.IsInit())

Source/Core/UICommon/UICommon.h

@@ -6,6 +6,7 @@
 #include <string>
 
 #include "Common/CommonTypes.h"
+#include "Common/HookableEvent.h"
 
 struct WindowSystemInfo;
 
@@ -14,6 +15,10 @@ namespace UICommon
 void Init();
 void Shutdown();
 
+// Triggered from the Host-thread on Android before a potential process termination.
+[[nodiscard]] Common::EventHook AddFlushUnsavedDataCallback(std::function<void()> callback);
+void FlushUnsavedData();
+
 void InitControllers(const WindowSystemInfo& wsi);
 void ShutdownControllers();
 

Source/UnitTests/Common/MutexTest.cpp

@@ -6,9 +6,11 @@
 #include <algorithm>
 #include <chrono>
 #include <mutex>
+#include <shared_mutex>
 #include <thread>
 
 #include "Common/Mutex.h"
+#include "Common/TransferableSharedMutex.h"
 
 template <typename MutexType>
 static void DoAtomicMutexTests(const char mutex_name[])
@@ -100,3 +102,123 @@ TEST(Mutex, AtomicMutex)
   DoAtomicMutexTests<Common::AtomicMutex>("AtomicMutex");
   DoAtomicMutexTests<Common::SpinMutex>("SpinMutex");
 }
+
+TEST(Mutex, TransferableSharedMutex)
+{
+  Common::TransferableSharedMutex work_mutex;
+
+  bool worker_done = false;
+
+  static constexpr auto SLEEP_TIME = std::chrono::microseconds{1};
+
+  // lock() on main thread, unlock() on worker thread.
+  std::thread thread{[&, lk = std::unique_lock{work_mutex}] {
+    std::this_thread::sleep_for(SLEEP_TIME);
+    worker_done = true;
+  }};
+
+  // lock() waits for the thread to unlock().
+  {
+    std::lock_guard lk{work_mutex};
+    EXPECT_TRUE(worker_done);
+  }
+
+  thread.join();
+
+  // Prevent below workers from incrementing `done_count`.
+  Common::TransferableSharedMutex done_mutex;
+  std::unique_lock done_lk{done_mutex};
+
+  // try_*() fails when holding an exclusive lock.
+  EXPECT_FALSE(done_mutex.try_lock());
+  EXPECT_FALSE(done_mutex.try_lock_shared());
+
+  static constexpr int THREAD_COUNT = 4;
+  static constexpr int REPEAT_COUNT = 100;
+  static constexpr int TOTAL_ITERATIONS = THREAD_COUNT * REPEAT_COUNT;
+
+  std::atomic<int> work_count = 0;
+  std::atomic<int> done_count = 0;
+  int additional_work_count = 0;
+
+  std::atomic<int> try_lock_fail_count = 0;
+  std::atomic<int> try_lock_shared_fail_count = 0;
+
+  std::vector<std::thread> threads(THREAD_COUNT);
+  for (auto& t : threads)
+  {
+    // lock_shared() multiple times on main thread.
+    t = std::thread{[&, work_lk = std::shared_lock{work_mutex}]() mutable {
+      std::this_thread::sleep_for(SLEEP_TIME);
+
+      // try_lock() fails after lock_shared().
+      EXPECT_FALSE(work_mutex.try_lock());
+
+      // Main thread already holds done_mutex.
+      EXPECT_FALSE(done_mutex.try_lock());
+      EXPECT_FALSE(done_mutex.try_lock_shared());
+
+      ++work_count;
+
+      // Signal work is done.
+      work_lk.unlock();
+
+      // lock_shared() blocks until main thread unlock()s.
+      {
+        std::shared_lock lk{done_mutex};
+        ++done_count;
+      }
+
+      // Contesting all of [try_]lock[_shared] doesn't explode.
+      for (int i = 0; i != REPEAT_COUNT; ++i)
+      {
+        while (!work_mutex.try_lock())
+        {
+          try_lock_fail_count.fetch_add(1, std::memory_order_relaxed);
+        }
+        work_mutex.unlock();
+
+        while (!work_mutex.try_lock_shared())
+        {
+          try_lock_shared_fail_count.fetch_add(1, std::memory_order_relaxed);
+        }
+        work_mutex.unlock_shared();
+        {
+          std::lock_guard lk{work_mutex};
+          ++additional_work_count;
+        }
+        std::shared_lock lk{work_mutex};
+      }
+    }};
+  }
+
+  // lock() waits for threads to unlock_shared().
+  {
+    std::lock_guard lk{work_mutex};
+    EXPECT_EQ(work_count.load(std::memory_order_relaxed), THREAD_COUNT);
+  }
+
+  std::this_thread::sleep_for(SLEEP_TIME);
+
+  // The threads are still blocking on done_mutex.
+  EXPECT_EQ(done_count, 0);
+
+  done_lk.unlock();
+  std::ranges::for_each(threads, &std::thread::join);
+
+  // The threads finished.
+  EXPECT_EQ(done_count, THREAD_COUNT);
+
+  EXPECT_EQ(additional_work_count, TOTAL_ITERATIONS);
+
+  GTEST_LOG_(INFO) << "try_lock() failure %: "
+                   << (try_lock_fail_count * 100.0 / (TOTAL_ITERATIONS + try_lock_fail_count));
+
+  GTEST_LOG_(INFO) << "try_lock_shared() failure %: "
+                   << (try_lock_shared_fail_count * 100.0 /
+                       (TOTAL_ITERATIONS + try_lock_shared_fail_count));
+
+  // Things are still sane after contesting in worker threads.
+  done_lk.lock();
+  std::lock_guard lk{work_mutex};
+}