Merge pull request #11103 from JosJuice/android-gamefilecache-not-null

Android: Allocate GameFileCache on GUI thread

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

@@ -74,7 +74,7 @@ public class AppLinkActivity extends FragmentActivity
     });
 
     DirectoryInitialization.start(this);
-    GameFileCacheManager.startLoad(this);
+    GameFileCacheManager.startLoad();
   }
 
   /**

Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/model/Settings.java

@@ -233,7 +233,7 @@ public class Settings implements Closeable
       if (mLoadedRecursiveIsoPathsValue != BooleanSetting.MAIN_RECURSIVE_ISO_PATHS.getBoolean(this))
       {
         // Refresh game library
-        GameFileCacheManager.startRescan(context);
+        GameFileCacheManager.startRescan();
       }
     }
     else

Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/model/GameFileCache.java

@@ -109,11 +109,11 @@ public class GameFileCache
 
   public static native String[] getAllGamePaths(String[] folderPaths, boolean recursiveScan);
 
-  public native int getSize();
+  public synchronized native int getSize();
 
-  public native GameFile[] getAllGames();
+  public synchronized native GameFile[] getAllGames();
 
-  public native GameFile addOrGet(String gamePath);
+  public synchronized native GameFile addOrGet(String gamePath);
 
   /**
    * Sets the list of games to cache.
@@ -123,7 +123,7 @@ public class GameFileCache
    *
    * @return true if the cache was modified
    */
-  public native boolean update(String[] gamePaths);
+  public synchronized native boolean update(String[] gamePaths);
 
   /**
    * For each game that already is in the cache, scans the folder that contains the game
@@ -131,9 +131,9 @@ public class GameFileCache
    *
    * @return true if the cache was modified
    */
-  public native boolean updateAdditionalMetadata();
+  public synchronized native boolean updateAdditionalMetadata();
 
-  public native boolean load();
+  public synchronized native boolean load();
 
-  public native boolean save();
+  public synchronized native boolean save();
 }

Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/services/GameFileCacheManager.java

@@ -2,8 +2,6 @@
 
 package org.dolphinemu.dolphinemu.services;
 
-import android.content.Context;
-
 import androidx.lifecycle.LiveData;
 import androidx.lifecycle.MutableLiveData;
 
@@ -23,14 +21,15 @@ import java.util.concurrent.Executors;
  */
 public final class GameFileCacheManager
 {
-  private static GameFileCache gameFileCache = null;
+  private static GameFileCache sGameFileCache = null;
-  private static final MutableLiveData<GameFile[]> gameFiles =
+  private static final MutableLiveData<GameFile[]> sGameFiles =
           new MutableLiveData<>(new GameFile[]{});
-  private static boolean runRescanAfterLoad = false;
+  private static boolean sFirstLoadDone = false;
+  private static boolean sRunRescanAfterLoad = false;
 
-  private static final ExecutorService executor = Executors.newFixedThreadPool(1);
+  private static final ExecutorService sExecutor = Executors.newFixedThreadPool(1);
-  private static final MutableLiveData<Boolean> loadInProgress = new MutableLiveData<>(false);
+  private static final MutableLiveData<Boolean> sLoadInProgress = new MutableLiveData<>(false);
-  private static final MutableLiveData<Boolean> rescanInProgress = new MutableLiveData<>(false);
+  private static final MutableLiveData<Boolean> sRescanInProgress = new MutableLiveData<>(false);
 
   private GameFileCacheManager()
   {
@@ -38,12 +37,12 @@ public final class GameFileCacheManager
 
   public static LiveData<GameFile[]> getGameFiles()
   {
-    return gameFiles;
+    return sGameFiles;
   }
 
   public static List<GameFile> getGameFilesForPlatform(Platform platform)
   {
-    GameFile[] allGames = gameFiles.getValue();
+    GameFile[] allGames = sGameFiles.getValue();
     ArrayList<GameFile> platformGames = new ArrayList<>();
     for (GameFile game : allGames)
     {
@@ -57,7 +56,7 @@ public final class GameFileCacheManager
 
   public static GameFile getGameFileByGameId(String gameId)
   {
-    GameFile[] allGames = gameFiles.getValue();
+    GameFile[] allGames = sGameFiles.getValue();
     for (GameFile game : allGames)
     {
       if (game.getGameId().equals(gameId))
@@ -72,7 +71,7 @@ public final class GameFileCacheManager
   {
     GameFile matchWithoutRevision = null;
 
-    GameFile[] allGames = gameFiles.getValue();
+    GameFile[] allGames = sGameFiles.getValue();
     for (GameFile otherGame : allGames)
     {
       if (game.getGameId().equals(otherGame.getGameId()) &&
@@ -102,7 +101,7 @@ public final class GameFileCacheManager
    */
   public static LiveData<Boolean> isLoading()
   {
-    return loadInProgress;
+    return sLoadInProgress;
   }
 
   /**
@@ -110,12 +109,12 @@ public final class GameFileCacheManager
    */
   public static LiveData<Boolean> isRescanning()
   {
-    return rescanInProgress;
+    return sRescanInProgress;
   }
 
   public static boolean isLoadingOrRescanning()
   {
-    return loadInProgress.getValue() || rescanInProgress.getValue();
+    return sLoadInProgress.getValue() || sRescanInProgress.getValue();
   }
 
   /**
@@ -123,13 +122,15 @@ public final class GameFileCacheManager
    * if the games are still present in the user's configured folders.
    * If this has already been called, calling it again has no effect.
    */
-  public static void startLoad(Context context)
+  public static void startLoad()
   {
-    if (!loadInProgress.getValue())
+    createGameFileCacheIfNeeded();
+
+    if (!sLoadInProgress.getValue())
     {
-      loadInProgress.setValue(true);
+      sLoadInProgress.setValue(true);
       new AfterDirectoryInitializationRunner().runWithoutLifecycle(
-              () -> executor.execute(GameFileCacheManager::load));
+              () -> sExecutor.execute(GameFileCacheManager::load));
     }
   }
 
@@ -139,13 +140,15 @@ public final class GameFileCacheManager
    * If loading the game file cache hasn't started or hasn't finished,
    * the execution of this will be postponed until it finishes.
    */
-  public static void startRescan(Context context)
+  public static void startRescan()
   {
-    if (!rescanInProgress.getValue())
+    createGameFileCacheIfNeeded();
+
+    if (!sRescanInProgress.getValue())
     {
-      rescanInProgress.setValue(true);
+      sRescanInProgress.setValue(true);
       new AfterDirectoryInitializationRunner().runWithoutLifecycle(
-              () -> executor.execute(GameFileCacheManager::rescan));
+              () -> sExecutor.execute(GameFileCacheManager::rescan));
     }
   }
 
@@ -153,8 +156,8 @@ public final class GameFileCacheManager
   {
     // Common case: The game is in the cache, so just grab it from there.
     // (Actually, addOrGet already checks for this case, but we want to avoid calling it if possible
-    // because onHandleIntent may hold a lock on gameFileCache for extended periods of time.)
+    // because the executor thread may hold a lock on sGameFileCache for extended periods of time.)
-    GameFile[] allGames = gameFiles.getValue();
+    GameFile[] allGames = sGameFiles.getValue();
     for (GameFile game : allGames)
     {
       if (game.getPath().equals(gamePath))
@@ -165,10 +168,8 @@ public final class GameFileCacheManager
 
     // Unusual case: The game wasn't found in the cache.
     // Scan the game and add it to the cache so that we can return it.
-    synchronized (gameFileCache)
+    createGameFileCacheIfNeeded();
-    {
+    return sGameFileCache.addOrGet(gamePath);
-      return gameFileCache.addOrGet(gamePath);
-    }
   }
 
   /**
@@ -178,30 +179,26 @@ public final class GameFileCacheManager
    */
   private static void load()
   {
-    if (gameFileCache == null)
+    if (!sFirstLoadDone)
     {
-      GameFileCache temp = new GameFileCache();
+      sFirstLoadDone = true;
-      synchronized (temp)
+      sGameFileCache.load();
+      if (sGameFileCache.getSize() != 0)
       {
-        gameFileCache = temp;
+        updateGameFileArray();
-        gameFileCache.load();
-        if (gameFileCache.getSize() != 0)
-        {
-          updateGameFileArray();
-        }
       }
     }
 
-    if (runRescanAfterLoad)
+    if (sRunRescanAfterLoad)
     {
-      rescanInProgress.postValue(true);
+      sRescanInProgress.postValue(true);
     }
 
-    loadInProgress.postValue(false);
+    sLoadInProgress.postValue(false);
 
-    if (runRescanAfterLoad)
+    if (sRunRescanAfterLoad)
     {
-      runRescanAfterLoad = false;
+      sRunRescanAfterLoad = false;
       rescan();
     }
   }
@@ -214,25 +211,21 @@ public final class GameFileCacheManager
    */
   private static void rescan()
   {
-    if (gameFileCache == null)
+    if (!sFirstLoadDone)
     {
-      runRescanAfterLoad = true;
+      sRunRescanAfterLoad = true;
     }
     else
     {
       String[] gamePaths = GameFileCache.getAllGamePaths();
 
-      boolean changed;
+      boolean changed = sGameFileCache.update(gamePaths);
-      synchronized (gameFileCache)
-      {
-        changed = gameFileCache.update(gamePaths);
-      }
       if (changed)
       {
         updateGameFileArray();
       }
 
-      boolean additionalMetadataChanged = gameFileCache.updateAdditionalMetadata();
+      boolean additionalMetadataChanged = sGameFileCache.updateAdditionalMetadata();
       if (additionalMetadataChanged)
       {
         updateGameFileArray();
@@ -240,17 +233,29 @@ public final class GameFileCacheManager
 
       if (changed || additionalMetadataChanged)
       {
-        gameFileCache.save();
+        sGameFileCache.save();
       }
     }
 
-    rescanInProgress.postValue(false);
+    sRescanInProgress.postValue(false);
   }
 
   private static void updateGameFileArray()
   {
-    GameFile[] gameFilesTemp = gameFileCache.getAllGames();
+    GameFile[] gameFilesTemp = sGameFileCache.getAllGames();
     Arrays.sort(gameFilesTemp, (lhs, rhs) -> lhs.getTitle().compareToIgnoreCase(rhs.getTitle()));
-    gameFiles.postValue(gameFilesTemp);
+    sGameFiles.postValue(gameFilesTemp);
+  }
+
+  private static void createGameFileCacheIfNeeded()
+  {
+    // Creating the GameFileCache in the static initializer may be unsafe, because GameFileCache
+    // relies on native code, and the native library isn't loaded right when the app starts.
+    // We create it here instead.
+
+    if (sGameFileCache == null)
+    {
+      sGameFileCache = new GameFileCache();
+    }
   }
 }

Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/ui/main/MainActivity.java

@@ -301,7 +301,7 @@ public final class MainActivity extends AppCompatActivity
   public void onRefresh()
   {
     setRefreshing(true);
-    GameFileCacheManager.startRescan(this);
+    GameFileCacheManager.startRescan();
   }
 
   /**
@@ -368,7 +368,7 @@ public final class MainActivity extends AppCompatActivity
     mViewPager.setCurrentItem(IntSetting.MAIN_LAST_PLATFORM_TAB.getIntGlobal());
 
     showGames();
-    GameFileCacheManager.startLoad(this);
+    GameFileCacheManager.startLoad();
   }
 
   @Override

Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/ui/main/MainPresenter.java

@@ -96,7 +96,7 @@ public final class MainPresenter
 
       case R.id.menu_refresh:
         mView.setRefreshing(true);
-        GameFileCacheManager.startRescan(activity);
+        GameFileCacheManager.startRescan();
         return true;
 
       case R.id.button_add_directory:
@@ -146,7 +146,7 @@ public final class MainPresenter
 
     if (sShouldRescanLibrary)
     {
-      GameFileCacheManager.startRescan(mActivity);
+      GameFileCacheManager.startRescan();
     }
 
     sShouldRescanLibrary = true;

Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/ui/main/TvMainActivity.java

@@ -79,7 +79,7 @@ public final class TvMainActivity extends FragmentActivity
     if (DirectoryInitialization.shouldStart(this))
     {
       DirectoryInitialization.start(this);
-      GameFileCacheManager.startLoad(this);
+      GameFileCacheManager.startLoad();
     }
 
     mPresenter.onResume();
@@ -292,7 +292,7 @@ public final class TvMainActivity extends FragmentActivity
       }
 
       DirectoryInitialization.start(this);
-      GameFileCacheManager.startLoad(this);
+      GameFileCacheManager.startLoad();
     }
   }
 
@@ -303,7 +303,7 @@ public final class TvMainActivity extends FragmentActivity
   public void onRefresh()
   {
     setRefreshing(true);
-    GameFileCacheManager.startRescan(this);
+    GameFileCacheManager.startRescan();
   }
 
   private void buildRowsAdapter()
@@ -313,7 +313,7 @@ public final class TvMainActivity extends FragmentActivity
 
     if (!DirectoryInitialization.isWaitingForWriteAccess(this))
     {
-      GameFileCacheManager.startLoad(this);
+      GameFileCacheManager.startLoad();
     }
 
     for (Platform platform : Platform.values())

Source/CMakeLists.txt

@@ -13,8 +13,6 @@ if(CMAKE_SYSTEM_NAME MATCHES "Windows")
   add_definitions(-D_CRT_SECURE_NO_DEPRECATE)
   add_definitions(-D_CRT_NONSTDC_NO_WARNINGS)
   add_definitions(-D_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING)
-  # The replacement for the old atomic shared_ptr functions was added in C++20, so we can't use it yet
-  add_definitions(-D_SILENCE_CXX20_OLD_SHARED_PTR_ATOMIC_SUPPORT_DEPRECATION_WARNING)
 endif()
 
 if (NOT MSVC)

Source/Core/UICommon/GameFileCache.cpp

@@ -4,7 +4,6 @@
 #include "UICommon/GameFileCache.h"
 
 #include <algorithm>
-#include <atomic>
 #include <cstddef>
 #include <functional>
 #include <list>
@@ -204,7 +203,7 @@ bool GameFileCache::UpdateAdditionalMetadata(std::shared_ptr<GameFile>* game_fil
   if (custom_cover_changed)
     copy->CustomCoverCommit();
 
-  std::atomic_store(game_file, std::move(copy));
+  *game_file = std::move(copy);
 
   return true;
 }

Source/VSProps/Base.Dolphin.props

@@ -29,8 +29,6 @@
       <PreprocessorDefinitions>_WINSOCK_DEPRECATED_NO_WARNINGS;%(PreprocessorDefinitions)</PreprocessorDefinitions>
       <!--Currently needed for some code in StringUtil used only on Android-->
       <PreprocessorDefinitions>_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING;%(PreprocessorDefinitions)</PreprocessorDefinitions>
-      <!--The replacement for the old atomic shared_ptr functions was added in C++20, so we can't use it yet-->
-      <PreprocessorDefinitions>_SILENCE_CXX20_OLD_SHARED_PTR_ATOMIC_SUPPORT_DEPRECATION_WARNING;%(PreprocessorDefinitions)</PreprocessorDefinitions>
 
       <!--Dolphin-specific definitions-->
       <PreprocessorDefinitions Condition="'$(Platform)'=='x64'">_ARCH_64=1;_M_X86=1;_M_X86_64=1;%(PreprocessorDefinitions)</PreprocessorDefinitions>