Merge pull request #9229 from JosJuice/android-emulationactivity-finish

Android: Handle failed boots correctly

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

@@ -12,6 +12,8 @@ import android.util.DisplayMetrics;
 import android.view.Surface;
 import android.widget.Toast;
 
+import androidx.fragment.app.FragmentManager;
+
 import org.dolphinemu.dolphinemu.activities.EmulationActivity;
 import org.dolphinemu.dolphinemu.dialogs.AlertMessage;
 import org.dolphinemu.dolphinemu.utils.CompressCallback;
@@ -403,15 +405,13 @@ public final class NativeLibrary
    */
   public static native void StopEmulation();
 
-  public static native boolean IsBooting();
-
-  public static native void WaitUntilDoneBooting();
-
   /**
    * Returns true if emulation is running (or is paused).
    */
   public static native boolean IsRunning();
 
+  public static native boolean IsRunningAndStarted();
+
   /**
    * Enables or disables CPU block profiling
    *
@@ -487,7 +487,7 @@ public final class NativeLibrary
   private static native String GetCurrentTitleDescriptionUnchecked();
 
   public static boolean displayAlertMsg(final String caption, final String text,
-          final boolean yesNo, final boolean isWarning)
+          final boolean yesNo, final boolean isWarning, final boolean nonBlocking)
   {
     Log.error("[NativeLibrary] Alert: " + text);
     final EmulationActivity emulationActivity = sEmulationActivity.get();
@@ -498,10 +498,9 @@ public final class NativeLibrary
     }
     else
     {
-      // AlertMessages while the core is booting will deadlock if WaitUntilDoneBooting is called.
+      // We can't use AlertMessages unless we have a non-null activity reference
-      // We also can't use AlertMessages unless we have a non-null activity reference.
+      // and are allowed to block. As a fallback, we can use toasts.
-      // As a fallback, we use toasts instead.
+      if (emulationActivity == null || nonBlocking)
-      if (emulationActivity == null || IsBooting())
       {
         new Handler(Looper.getMainLooper()).post(
                 () -> Toast.makeText(DolphinApplication.getAppContext(), text, Toast.LENGTH_LONG)
@@ -511,9 +510,22 @@ public final class NativeLibrary
       {
         sIsShowingAlertMessage = true;
 
-        emulationActivity.runOnUiThread(
+        emulationActivity.runOnUiThread(() ->
-                () -> AlertMessage.newInstance(caption, text, yesNo, isWarning)
+        {
-                        .show(emulationActivity.getSupportFragmentManager(), "AlertMessage"));
+          FragmentManager fragmentManager = emulationActivity.getSupportFragmentManager();
+          if (fragmentManager.isStateSaved())
+          {
+            // The activity is being destroyed, so we can't use it to display an AlertMessage.
+            // Fall back to a toast.
+            Toast.makeText(emulationActivity, text, Toast.LENGTH_LONG).show();
+            NotifyAlertMessageLock();
+          }
+          else
+          {
+            AlertMessage.newInstance(caption, text, yesNo, isWarning)
+                    .show(fragmentManager, "AlertMessage");
+          }
+        });
 
         // Wait for the lock to notify that it is complete.
         synchronized (sAlertMessageLock)
@@ -563,6 +575,20 @@ public final class NativeLibrary
     sEmulationActivity.clear();
   }
 
+  public static void finishEmulationActivity()
+  {
+    final EmulationActivity emulationActivity = sEmulationActivity.get();
+    if (emulationActivity == null)
+    {
+      Log.warning("[NativeLibrary] EmulationActivity is null.");
+    }
+    else
+    {
+      Log.verbose("[NativeLibrary] Finishing EmulationActivity.");
+      emulationActivity.runOnUiThread(emulationActivity::finish);
+    }
+  }
+
   public static void updateTouchPointer()
   {
     final EmulationActivity emulationActivity = sEmulationActivity.get();

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

@@ -81,10 +81,12 @@ public final class EmulationActivity extends AppCompatActivity
   private String[] mPaths;
   private boolean mIgnoreWarnings;
   private static boolean sUserPausedEmulation;
+  private boolean mMenuToastShown;
 
   public static final String EXTRA_SELECTED_GAMES = "SelectedGames";
   public static final String EXTRA_IGNORE_WARNINGS = "IgnoreWarnings";
   public static final String EXTRA_USER_PAUSED_EMULATION = "sUserPausedEmulation";
+  public static final String EXTRA_MENU_TOAST_SHOWN = "MenuToastShown";
 
   @Retention(SOURCE)
   @IntDef({MENU_ACTION_EDIT_CONTROLS_PLACEMENT, MENU_ACTION_TOGGLE_CONTROLS, MENU_ACTION_ADJUST_SCALE,
@@ -212,8 +214,8 @@ public final class EmulationActivity extends AppCompatActivity
       mPaths = gameToEmulate.getStringArrayExtra(EXTRA_SELECTED_GAMES);
       mIgnoreWarnings = gameToEmulate.getBooleanExtra(EXTRA_IGNORE_WARNINGS, false);
       sUserPausedEmulation = gameToEmulate.getBooleanExtra(EXTRA_USER_PAUSED_EMULATION, false);
+      mMenuToastShown = false;
       activityRecreated = false;
-      Toast.makeText(this, R.string.emulation_menu_help, Toast.LENGTH_LONG).show();
     }
     else
     {
@@ -260,8 +262,9 @@ public final class EmulationActivity extends AppCompatActivity
       mEmulationFragment.saveTemporaryState();
     }
     outState.putStringArray(EXTRA_SELECTED_GAMES, mPaths);
-    outState.putBoolean(EXTRA_USER_PAUSED_EMULATION, mIgnoreWarnings);
+    outState.putBoolean(EXTRA_IGNORE_WARNINGS, mIgnoreWarnings);
     outState.putBoolean(EXTRA_USER_PAUSED_EMULATION, sUserPausedEmulation);
+    outState.putBoolean(EXTRA_MENU_TOAST_SHOWN, mMenuToastShown);
     super.onSaveInstanceState(outState);
   }
 
@@ -270,6 +273,7 @@ public final class EmulationActivity extends AppCompatActivity
     mPaths = savedInstanceState.getStringArray(EXTRA_SELECTED_GAMES);
     mIgnoreWarnings = savedInstanceState.getBoolean(EXTRA_IGNORE_WARNINGS);
     sUserPausedEmulation = savedInstanceState.getBoolean(EXTRA_USER_PAUSED_EMULATION);
+    mMenuToastShown = savedInstanceState.getBoolean(EXTRA_MENU_TOAST_SHOWN);
   }
 
   @Override
@@ -306,6 +310,13 @@ public final class EmulationActivity extends AppCompatActivity
 
   public void onTitleChanged()
   {
+    if (!mMenuToastShown)
+    {
+      // The reason why this doesn't run earlier is because we want to be sure the boot succeeded.
+      Toast.makeText(this, R.string.emulation_menu_help, Toast.LENGTH_LONG).show();
+      mMenuToastShown = true;
+    }
+
     setTitle(NativeLibrary.GetCurrentTitleDescription());
     updateMotionListener();
 
@@ -342,7 +353,6 @@ public final class EmulationActivity extends AppCompatActivity
     if (keyCode == KeyEvent.KEYCODE_BACK)
     {
       mEmulationFragment.stopEmulation();
-      finish();
       return true;
     }
     return super.onKeyLongPress(keyCode, event);
@@ -617,7 +627,6 @@ public final class EmulationActivity extends AppCompatActivity
 
       case MENU_ACTION_EXIT:
         mEmulationFragment.stopEmulation();
-        finish();
         break;
     }
   }

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

@@ -330,16 +330,6 @@ public final class EmulationFragment extends Fragment implements SurfaceHolder.C
         mSurface = null;
         Log.debug("[EmulationFragment] Surface destroyed.");
 
-        if (state != State.STOPPED && !NativeLibrary.IsShowingAlertMessage())
-        {
-          // In order to avoid dereferencing nullptr, we must not destroy the surface while booting
-          // the core, so wait here if necessary. An easy (but not 100% consistent) way to reach
-          // this method while the core is booting is by having landscape orientation lock enabled
-          // and starting emulation while the phone is in portrait mode, leading to the activity
-          // being recreated very soon after NativeLibrary.Run has been called.
-          NativeLibrary.WaitUntilDoneBooting();
-        }
-
         NativeLibrary.SurfaceDestroyed();
       }
     }

Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/overlay/InputOverlay.java

@@ -148,7 +148,7 @@ public final class InputOverlay extends SurfaceView implements OnTouchListener
   public void initTouchPointer()
   {
     // Check if we have all the data we need yet
-    boolean aspectRatioAvailable = NativeLibrary.IsRunning() && !NativeLibrary.IsBooting();
+    boolean aspectRatioAvailable = NativeLibrary.IsRunningAndStarted();
     if (!aspectRatioAvailable || mSurfacePosition == null)
       return;
 

Source/Android/jni/AndroidCommon/IDCache.cpp

@@ -15,6 +15,7 @@ static jmethodID s_display_alert_msg;
 static jmethodID s_do_rumble;
 static jmethodID s_update_touch_pointer;
 static jmethodID s_on_title_changed;
+static jmethodID s_finish_emulation_activity;
 
 static jclass s_game_file_class;
 static jfieldID s_game_file_pointer;
@@ -94,6 +95,11 @@ jmethodID GetOnTitleChanged()
   return s_on_title_changed;
 }
 
+jmethodID GetFinishEmulationActivity()
+{
+  return s_finish_emulation_activity;
+}
+
 jclass GetAnalyticsClass()
 {
   return s_analytics_class;
@@ -216,11 +222,13 @@ jint JNI_OnLoad(JavaVM* vm, void* reserved)
   const jclass native_library_class = env->FindClass("org/dolphinemu/dolphinemu/NativeLibrary");
   s_native_library_class = reinterpret_cast<jclass>(env->NewGlobalRef(native_library_class));
   s_display_alert_msg = env->GetStaticMethodID(s_native_library_class, "displayAlertMsg",
-                                               "(Ljava/lang/String;Ljava/lang/String;ZZ)Z");
+                                               "(Ljava/lang/String;Ljava/lang/String;ZZZ)Z");
   s_do_rumble = env->GetStaticMethodID(s_native_library_class, "rumble", "(ID)V");
   s_update_touch_pointer =
       env->GetStaticMethodID(s_native_library_class, "updateTouchPointer", "()V");
   s_on_title_changed = env->GetStaticMethodID(s_native_library_class, "onTitleChanged", "()V");
+  s_finish_emulation_activity =
+      env->GetStaticMethodID(s_native_library_class, "finishEmulationActivity", "()V");
   env->DeleteLocalRef(native_library_class);
 
   const jclass game_file_class = env->FindClass("org/dolphinemu/dolphinemu/model/GameFile");

Source/Android/jni/AndroidCommon/IDCache.h

@@ -15,6 +15,7 @@ jmethodID GetDisplayAlertMsg();
 jmethodID GetDoRumble();
 jmethodID GetUpdateTouchPointer();
 jmethodID GetOnTitleChanged();
+jmethodID GetFinishEmulationActivity();
 
 jclass GetAnalyticsClass();
 jmethodID GetSendAnalyticsReport();

Source/Android/jni/MainAndroid.cpp

@@ -77,7 +77,12 @@ ANativeWindow* s_surf;
 // sequentially for access.
 std::mutex s_host_identity_lock;
 Common::Event s_update_main_frame_event;
-Common::Event s_emulation_end_event;
+
+// This exists to prevent surfaces from being destroyed during the boot process,
+// as that can lead to the boot process dereferencing nullptr.
+std::mutex s_surface_lock;
+bool s_need_nonblocking_alert_msg;
+
 bool s_have_wm_user_stop = false;
 bool s_game_metadata_is_valid = false;
 }  // Anonymous namespace
@@ -160,9 +165,10 @@ static bool MsgAlert(const char* caption, const char* text, bool yes_no, Common:
   JNIEnv* env = IDCache::GetEnvForThread();
 
   // Execute the Java method.
-  jboolean result = env->CallStaticBooleanMethod(
+  jboolean result =
-      IDCache::GetNativeLibraryClass(), IDCache::GetDisplayAlertMsg(), ToJString(env, caption),
+      env->CallStaticBooleanMethod(IDCache::GetNativeLibraryClass(), IDCache::GetDisplayAlertMsg(),
-      ToJString(env, text), yes_no ? JNI_TRUE : JNI_FALSE, style == Common::MsgType::Warning);
+                                   ToJString(env, caption), ToJString(env, text), yes_no,
+                                   style == Common::MsgType::Warning, s_need_nonblocking_alert_msg);
 
   return result != JNI_FALSE;
 }
@@ -210,33 +216,22 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_PauseEmulati
 
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_StopEmulation(JNIEnv*, jclass)
 {
-  {
+  std::lock_guard<std::mutex> guard(s_host_identity_lock);
-    std::lock_guard<std::mutex> guard(s_host_identity_lock);
+  Core::Stop();
-    s_emulation_end_event.Reset();
-    Core::Stop();
 
-    // Kick the waiting event
+  // Kick the waiting event
-    s_update_main_frame_event.Set();
+  s_update_main_frame_event.Set();
-  }
-
-  // Wait for shutdown, to avoid accessing the config at the same time as the shutdown code
-  s_emulation_end_event.Wait();
-}
-
-JNIEXPORT jboolean JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_IsBooting(JNIEnv*, jclass)
-{
-  return static_cast<jboolean>(Core::IsBooting());
-}
-
-JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_WaitUntilDoneBooting(JNIEnv*,
-                                                                                         jclass)
-{
-  Core::WaitUntilDoneBooting();
 }
 
 JNIEXPORT jboolean JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_IsRunning(JNIEnv*, jclass)
 {
-  return Core::IsRunning();
+  return static_cast<jboolean>(Core::IsRunning());
+}
+
+JNIEXPORT jboolean JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_IsRunningAndStarted(JNIEnv*,
+                                                                                            jclass)
+{
+  return static_cast<jboolean>(Core::IsRunningAndStarted());
 }
 
 JNIEXPORT jboolean JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_onGamePadEvent(
@@ -398,6 +393,8 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SurfaceChang
                                                                                    jclass,
                                                                                    jobject surf)
 {
+  std::lock_guard<std::mutex> guard(s_surface_lock);
+
   s_surf = ANativeWindow_fromSurface(env, surf);
   if (s_surf == nullptr)
     __android_log_print(ANDROID_LOG_ERROR, DOLPHIN_TAG, "Error: Surface is null.");
@@ -409,6 +406,8 @@ JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SurfaceChang
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_SurfaceDestroyed(JNIEnv*,
                                                                                      jclass)
 {
+  std::lock_guard<std::mutex> guard(s_surface_lock);
+
   if (g_renderer)
     g_renderer->ChangeSurface(nullptr);
 
@@ -487,7 +486,7 @@ static void Run(JNIEnv* env, const std::vector<std::string>& paths,
   ASSERT(!paths.empty());
   __android_log_print(ANDROID_LOG_INFO, DOLPHIN_TAG, "Running : %s", paths[0].c_str());
 
-  std::unique_lock<std::mutex> guard(s_host_identity_lock);
+  std::unique_lock<std::mutex> host_identity_guard(s_host_identity_lock);
 
   WiimoteReal::InitAdapterClass();
 
@@ -500,24 +499,41 @@ static void Run(JNIEnv* env, const std::vector<std::string>& paths,
   WindowSystemInfo wsi(WindowSystemType::Android, nullptr, s_surf, s_surf);
   wsi.render_surface_scale = GetRenderSurfaceScale(env);
 
-  // No use running the loop when booting fails
+  s_need_nonblocking_alert_msg = true;
-  if (BootManager::BootCore(std::move(boot), wsi))
+  std::unique_lock<std::mutex> surface_guard(s_surface_lock);
+
+  bool successful_boot = BootManager::BootCore(std::move(boot), wsi);
+  if (successful_boot)
   {
     ButtonManager::Init(SConfig::GetInstance().GetGameID());
+
     static constexpr int TIMEOUT = 10000;
     static constexpr int WAIT_STEP = 25;
     int time_waited = 0;
     // A Core::CORE_ERROR state would be helpful here.
-    while (!Core::IsRunning() && time_waited < TIMEOUT && !s_have_wm_user_stop)
+    while (!Core::IsRunningAndStarted())
     {
+      if (time_waited >= TIMEOUT || s_have_wm_user_stop)
+      {
+        successful_boot = false;
+        break;
+      }
+
       std::this_thread::sleep_for(std::chrono::milliseconds(WAIT_STEP));
       time_waited += WAIT_STEP;
     }
-    while (Core::IsRunning())
+  }
+
+  s_need_nonblocking_alert_msg = false;
+  surface_guard.unlock();
+
+  if (successful_boot)
+  {
+    while (Core::IsRunningAndStarted())
     {
-      guard.unlock();
+      host_identity_guard.unlock();
       s_update_main_frame_event.Wait();
-      guard.lock();
+      host_identity_guard.lock();
       Core::HostDispatchJobs();
     }
   }
@@ -525,15 +541,10 @@ static void Run(JNIEnv* env, const std::vector<std::string>& paths,
   s_game_metadata_is_valid = false;
   Core::Shutdown();
   ButtonManager::Shutdown();
-  guard.unlock();
+  host_identity_guard.unlock();
 
-  if (s_surf)
+  env->CallStaticVoidMethod(IDCache::GetNativeLibraryClass(),
-  {
+                            IDCache::GetFinishEmulationActivity());
-    ANativeWindow_release(s_surf);
-    s_surf = nullptr;
-  }
-
-  s_emulation_end_event.Set();
 }
 
 JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_Run___3Ljava_lang_String_2(

Source/Core/Core/Core.cpp

@@ -102,7 +102,6 @@ static bool s_is_stopping = false;
 static bool s_hardware_initialized = false;
 static bool s_is_started = false;
 static Common::Flag s_is_booting;
-static Common::Event s_done_booting;
 static std::thread s_emu_thread;
 static StateChangedCallbackFunc s_on_state_changed_callback;
 
@@ -175,11 +174,6 @@ void DisplayMessage(std::string message, int time_in_ms)
   OSD::AddMessage(std::move(message), time_in_ms);
 }
 
-bool IsBooting()
-{
-  return s_is_booting.IsSet() || !s_hardware_initialized;
-}
-
 bool IsRunning()
 {
   return (GetState() != State::Uninitialized || s_hardware_initialized) && !s_is_stopping;
@@ -249,7 +243,6 @@ bool Init(std::unique_ptr<BootParameters> boot, const WindowSystemInfo& wsi)
   g_video_backend->PrepareWindow(prepared_wsi);
 
   // Start the emu thread
-  s_done_booting.Reset();
   s_is_booting.Set();
   s_emu_thread = std::thread(EmuThread, std::move(boot), prepared_wsi);
   return true;
@@ -435,7 +428,6 @@ static void EmuThread(std::unique_ptr<BootParameters> boot, WindowSystemInfo wsi
     s_on_state_changed_callback(State::Starting);
   Common::ScopeGuard flag_guard{[] {
     s_is_booting.Clear();
-    s_done_booting.Set();
     s_is_started = false;
     s_is_stopping = false;
     s_wants_determinism = false;
@@ -568,7 +560,6 @@ static void EmuThread(std::unique_ptr<BootParameters> boot, WindowSystemInfo wsi
   // The hardware is initialized.
   s_hardware_initialized = true;
   s_is_booting.Clear();
-  s_done_booting.Set();
 
   // Set execution state to known values (CPU/FIFO/Audio Paused)
   CPU::Break();
@@ -692,12 +683,6 @@ State GetState()
   return State::Uninitialized;
 }
 
-void WaitUntilDoneBooting()
-{
-  if (IsBooting())
-    s_done_booting.Wait();
-}
-
 static std::string GenerateScreenshotFolderPath()
 {
   const std::string& gameId = SConfig::GetInstance().GetGameID();

Source/Core/Core/Core.h

@@ -99,7 +99,6 @@ void UndeclareAsCPUThread();
 
 std::string StopMessage(bool main_thread, std::string_view message);
 
-bool IsBooting();
 bool IsRunning();
 bool IsRunningAndStarted();       // is running and the CPU loop has been entered
 bool IsRunningInCurrentThread();  // this tells us whether we are running in the CPU thread.
@@ -111,7 +110,6 @@ bool WantsDeterminism();
 // [NOT THREADSAFE] For use by Host only
 void SetState(State state);
 State GetState();
-void WaitUntilDoneBooting();
 
 void SaveScreenShot();
 void SaveScreenShot(std::string_view name);