Fix a race on startup that could clear a loaded save. (#12437)

I noticed the race when running the game in debug mode from IntelliJ, where it was consistently being hit for me when I would click the button to open a save right after startup.
This PR fixes it by ensuring the save game will only be loaded after the initial map load.

Also, splits the function to load a map vs. a game and updates call sites.

game-app/game-core/src/main/java/games/strategy/engine/framework/startup/ui/panels/main/game/selector/GameSelectorModel.java

@@ -18,6 +18,7 @@
 import java.util.List;
 import java.util.Observable;
 import java.util.Optional;
+import java.util.concurrent.CountDownLatch;
 import java.util.function.Predicate;
 import javax.annotation.Nullable;
 import lombok.Getter;
@@ -45,40 +46,50 @@ public class GameSelectorModel extends Observable implements GameSelector {
   @Setter @Getter private ClientModel clientModelForHostBots = null;
   private Optional<String> saveGameToLoad = Optional.empty();
 
+  // Don't load a save game before the startup task to load the initial map has run, else that task
+  // may "lose" the race and overwrite the loaded saved game.
+  private final CountDownLatch readyForSaveLoad = new CountDownLatch(1);
+
   public GameSelectorModel() {}
 
-  /**
-   * Loads game data by parsing a given file.
-   *
-   * @return True if successfully loaded, otherwise false.
-   */
-  public boolean load(final Path xmlFile) {
+  public boolean loadMap(Path xmlFile) {
+    ensureExists(xmlFile);
+    fileName = null;
+    GameData gameData = parseAndValidate(xmlFile);
+    if (gameData != null && gameData.getGameName() == null) {
+      gameData = null;
+    }
+    setGameData(gameData);
+    this.setDefaultGame(xmlFile, gameData);
+    return gameData != null;
+  }
+
+  public boolean loadSave(Path saveFile) {
+    try {
+      readyForSaveLoad.await();
+    } catch (InterruptedException e) {
+      return false;
+    }
+    ensureExists(saveFile);
+    final GameData newData = GameDataManager.loadGame(saveFile).orElse(null);
+    if (newData == null) {
+      return false;
+    }
+    newData.setSaveGameFileName(saveFile.getFileName().toString());
+    this.fileName = saveFile.getFileName().toString();
+    setGameData(newData);
+    return true;
+  }
+
+  public void setReadyForSaveLoad() {
+    readyForSaveLoad.countDown();
+  }
+
+  private void ensureExists(Path file) {
     Preconditions.checkArgument(
-        Files.exists(xmlFile),
+        Files.exists(file),
         "Programming error, expected file to have already been checked to exist: "
-            + xmlFile.toAbsolutePath());
-
-    // if the file name is xml, load it as a new game
-    if (xmlFile.getFileName().toString().toLowerCase().endsWith("xml")) {
-      fileName = null;
-      GameData gameData = parseAndValidate(xmlFile);
-      if (gameData != null && gameData.getGameName() == null) {
-        gameData = null;
-      }
-      setGameData(gameData);
-      this.setDefaultGame(xmlFile, gameData);
-      return gameData != null;
-    } else {
-      // try to load it as a saved game whatever the extension
-      final GameData newData = GameDataManager.loadGame(xmlFile).orElse(null);
-      if (newData == null) {
-        return false;
-      }
-      newData.setSaveGameFileName(xmlFile.getFileName().toString());
-      this.fileName = xmlFile.getFileName().toString();
-      setGameData(newData);
-      return true;
-    }
+            + file.toAbsolutePath());
   }
 
   private void setDefaultGame(@Nullable final Path xmlFile, @Nullable final GameData gameData) {
@@ -165,7 +176,7 @@ private void notifyObs() {
   /** Clears AI game over cache and loads default game in a new thread. */
   @Override
   public void onGameEnded() {
-    // clear out ai cached properties (this ended up being the best place to put it,
+    // clear out AI cached properties (this ended up being the best place to put it,
     // as we have definitely left a game at this point)
     GameShutdownRegistry.runShutdownActions();
     ThreadRunner.runInNewThread(this::loadDefaultGameSameThread);
@@ -192,7 +203,17 @@ public void loadDefaultGameSameThread() {
         .filter(Predicate.not(String::isBlank))
         .filter(GameSelectorModel::gameUriExistsOnFileSystem)
         .map(GameSelectorModel::pathFromGameUri)
-        .ifPresentOrElse(this::load, this::resetDefaultGame);
+        .ifPresentOrElse(
+            (file) -> {
+              // if the file name is xml, load it as a new game
+              if (file.getFileName().toString().toLowerCase().endsWith("xml")) {
+                loadMap(file);
+              } else {
+                // try to load it as a saved game whatever the extension
+                loadSave(file);
+              }
+            },
+            this::resetDefaultGame);
   }
 
   private static Path pathFromGameUri(final String gameUri) {

game-app/game-core/src/main/java/games/strategy/engine/framework/startup/ui/panels/main/game/selector/GameSelectorPanel.java

@@ -238,15 +238,15 @@ private static GridBagConstraints buildGridRow(final int x, final int y, final I
   private static GridBagConstraints buildGrid(
       final int x, final int y, final Insets insets, final int width) {
     final int gridHeight = 1;
-    final double weigthX = 0;
-    final double weigthY = 0;
+    final double weightX = 0;
+    final double weightY = 0;
     final int anchor = GridBagConstraints.WEST;
     final int fill = GridBagConstraints.NONE;
     final int ipadx = 0;
     final int ipady = 0;
 
     return new GridBagConstraints(
-        x, y, width, gridHeight, weigthX, weigthY, anchor, fill, insets, ipadx, ipady);
+        x, y, width, gridHeight, weightX, weightY, anchor, fill, insets, ipadx, ipady);
   }
 
   private void setOriginalPropertiesMap(final GameData data) {
@@ -367,7 +367,7 @@ public void loadSaveFile(final Path file) {
         .build()
         .run(
             () -> {
-              if (model.load(file)) {
+              if (model.loadSave(file)) {
                 setOriginalPropertiesMap(model.getGameData());
               }
             });
@@ -393,17 +393,10 @@ private void gameSelected(final Path gameFile) {
     BackgroundTaskRunner.runInBackground(
         "Loading map...",
         () -> {
-          model.load(gameFile);
-          // warning: NPE check is not to protect against concurrency, another thread could still
-          // null
-          // out game data.
-          // The NPE check is to protect against the case where there are errors loading game, in
-          // which case we'll have a null game data.
-          if (model.getGameData() != null) {
+          if (model.loadMap(gameFile)) {
             setOriginalPropertiesMap(model.getGameData());
             // only for new games, not saved games, we set the default options, and set them only
-            // once
-            // (the first time it is loaded)
+            // once (the first time it is loaded)
             gamePropertiesCache.loadCachedGamePropertiesInto(model.getGameData());
           }
         });

game-app/game-headed/src/main/java/org/triplea/game/client/HeadedGameRunner.java

@@ -142,15 +142,19 @@ private static void start() {
           headedServerSetupModel = new HeadedServerSetupModel(gameSelectorModel);
           MainFrame.buildMainFrame(headedServerSetupModel, gameSelectorModel);
           headedServerSetupModel.showSelectType();
-          ThreadRunner.runInNewThread(HeadedGameRunner::showMainFrame);
+          ThreadRunner.runInNewThread(
+              () -> {
+                showMainFrame();
+                gameSelectorModel.setReadyForSaveLoad();
+              });
         });
 
     UpdateChecks.launch();
   }
 
   /**
    * Sets the 'main frame' to visible. In this context the main frame is the initial welcome (launch
-   * lobby/single player game etc..) screen presented to GUI enabled clients.
+   * lobby/single player game etc.) screen presented to GUI enabled clients.
    */
   public static void showMainFrame() {
     GameShutdownRegistry.runShutdownActions();
@@ -169,7 +173,7 @@ public static void showMainFrame() {
       final String saveGameFileName = System.getProperty(TRIPLEA_GAME, "");
       if (!saveGameFileName.isEmpty()) {
         final Path saveGameFile = Path.of(saveGameFileName);
-        if (Files.exists(saveGameFile) && !gameSelectorModel.load(saveGameFile)) {
+        if (Files.exists(saveGameFile) && !gameSelectorModel.loadSave(saveGameFile)) {
           // abort launch if we failed to load the specified game
           return;
         }

game-app/game-headless/src/main/java/org/triplea/game/server/HeadlessGameServer.java

@@ -60,7 +60,7 @@ public synchronized void setGameMapTo(final String gameName) {
     log.info("Requested to change map to: " + gameName);
     // don't change mid-game and only if we have the game
     if (game == null && availableGames.hasGame(gameName)) {
-      gameSelectorModel.load(availableGames.findGameXmlPathByGameName(gameName).orElseThrow());
+      gameSelectorModel.loadMap(availableGames.findGameXmlPathByGameName(gameName).orElseThrow());
       log.info("Changed to game map: " + gameName);
     } else {
       log.info(
@@ -74,7 +74,7 @@ public synchronized void loadGameSave(final Path file) {
     Preconditions.checkArgument(
         Files.exists(file), "File must exist to load it: " + file.toAbsolutePath());
     // don't change mid-game
-    if (game == null && gameSelectorModel.load(file)) {
+    if (game == null && gameSelectorModel.loadSave(file)) {
       log.info("Changed to save: " + file.getFileName());
     }
   }

game-app/game-headless/src/main/java/org/triplea/game/server/HeadlessLaunchAction.java

@@ -67,7 +67,7 @@ public void handleGameInterruption(
     serverModel.setAllPlayersToNullNodes();
     final Path autoSaveFile = getAutoSaveFileUtils().getHeadlessAutoSaveFile();
     if (Files.exists(autoSaveFile)) {
-      gameSelectorModel.load(autoSaveFile);
+      gameSelectorModel.loadSave(autoSaveFile);
     }
   }