Fix memory leak.

Prevent nullpoint when finding backdoor into mod/replay vault.
ta-forever · Mar 23, 2021 · c0c4f39 · c0c4f39
1 parent 57bd4c6
commit c0c4f39
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 20 deletions.
diff --git a/src/main/java/com/faforever/client/game/GameDetailController.java b/src/main/java/com/faforever/client/game/GameDetailController.java
@@ -24,6 +24,8 @@
 import javafx.beans.WeakInvalidationListener;
 import javafx.beans.property.ReadOnlyObjectProperty;
 import javafx.beans.property.ReadOnlyObjectWrapper;
+import javafx.beans.value.ChangeListener;
+import javafx.beans.value.WeakChangeListener;
 import javafx.collections.ObservableMap;
 import javafx.event.ActionEvent;
 import javafx.scene.Node;
@@ -89,6 +91,16 @@ public class GameDetailController implements Controller<Pane> {
 
   @SuppressWarnings("FieldCanBeLocal")
   private InvalidationListener featuredModInvalidationListener;
+  private ChangeListener<GameStatus> currentGameStatusListener;
+  private ChangeListener<Boolean> gameRunningListener;
+
+  /* sever ties to external objects so that this instance can be garbage collected */
+  public void sever() {
+    if (gameTimeSinceStartUpdater !=null) {
+      gameTimeSinceStartUpdater.stop();
+    }
+    eventBus.unregister(this);
+  }
 
   public GameDetailController(I18n i18n, MapService mapService, ModService modService,
                               GameService gameService, PlayerService playerService,
@@ -111,13 +123,10 @@ public GameDetailController(I18n i18n, MapService mapService, ModService modServ
     weakTeamListener = new WeakInvalidationListener(teamsInvalidationListener);
     weakGameStatusListener = new WeakInvalidationListener(gameStatusInvalidationListener);
 
-    gameService.getCurrentGameStatusProperty().addListener((obs, newValue, oldValue) -> {
-      updateButtonsVisibility(gameService.getCurrentGame(), playerService.getCurrentPlayer().get());
-    });
-    gameService.gameRunningProperty().addListener((obs, newValue, oldValue) -> {
-      updateButtonsVisibility(gameService.getCurrentGame(), playerService.getCurrentPlayer().get());
-    });
-
+    currentGameStatusListener = (obs, newValue, oldValue) -> updateButtonsVisibility(gameService.getCurrentGame(), playerService.getCurrentPlayer().get());
+    gameRunningListener = (obs, newValue, oldValue) -> updateButtonsVisibility(gameService.getCurrentGame(), playerService.getCurrentPlayer().get());
+    gameService.getCurrentGameStatusProperty().addListener(new WeakChangeListener<>(currentGameStatusListener));
+    gameService.gameRunningProperty().addListener(new WeakChangeListener<>(gameRunningListener));
     eventBus.register(this);
   }
 

diff --git a/src/main/java/com/faforever/client/game/GameTileController.java b/src/main/java/com/faforever/client/game/GameTileController.java
@@ -19,6 +19,8 @@
 import javafx.animation.KeyFrame;
 import javafx.animation.Timeline;
 import javafx.application.Platform;
+import javafx.beans.value.ChangeListener;
+import javafx.beans.value.WeakChangeListener;
 import javafx.collections.ObservableMap;
 import javafx.event.ActionEvent;
 import javafx.scene.Node;
@@ -80,14 +82,19 @@ public class GameTileController implements Controller<Node> {
   public Button leaveButton;
   public Button startButton;
 
+  private ChangeListener<GameStatus> currentGameStatusListener;
+  private ChangeListener<Boolean> gameRunningListener;
+
   public void setOnSelectedListener(Consumer<Game> onSelectedListener) {
     this.onSelectedListener = onSelectedListener;
   }
 
-  public void stopGameStatusTimeUpdater() {
+  /* sever ties to external objects so that this instance can be garbage collected */
+  public void sever() {
     if (gameTimeSinceStartUpdater !=null) {
       gameTimeSinceStartUpdater.stop();
     }
+    eventBus.unregister(this);
   }
 
   public void initialize() {
@@ -96,12 +103,10 @@ public void initialize() {
     gameTypeLabel.managedProperty().bind(gameTypeLabel.visibleProperty());
     lockIconLabel.managedProperty().bind(lockIconLabel.visibleProperty());
 
-    gameService.getCurrentGameStatusProperty().addListener((obs, newValue, oldValue) -> {
-      updateButtonsVisibility(gameService.getCurrentGame(), playerService.getCurrentPlayer().get());
-    });
-    gameService.gameRunningProperty().addListener((obs, newValue, oldValue) -> {
-      updateButtonsVisibility(gameService.getCurrentGame(), playerService.getCurrentPlayer().get());
-    });
+    currentGameStatusListener = (obs, newValue, oldValue) -> updateButtonsVisibility(gameService.getCurrentGame(), playerService.getCurrentPlayer().get());
+    gameRunningListener = (obs, newValue, oldValue) -> updateButtonsVisibility(gameService.getCurrentGame(), playerService.getCurrentPlayer().get());
+    JavaFxUtil.addListener(gameService.getCurrentGameStatusProperty(), new WeakChangeListener<>(currentGameStatusListener));
+    JavaFxUtil.addListener(gameService.gameRunningProperty(), new WeakChangeListener<>(gameRunningListener));
 
     gameTimeSinceStartLabel.setVisible(false);
     gameTimeSinceStartUpdater = new Timeline(1,new KeyFrame(javafx.util.Duration.seconds(0), (ActionEvent event) -> {

diff --git a/src/main/java/com/faforever/client/game/GamesTilesContainerController.java b/src/main/java/com/faforever/client/game/GamesTilesContainerController.java
@@ -167,7 +167,7 @@ private void addGameCard(Game game) {
     root.setUserData(game);
     tiledFlowPane.getChildren().add(root);
     uidToGameCard.put(game.getId(), gameTileController);
-    
+
     root.setOnMouseEntered(event -> {
       gameTooltipController.setGame(game);
       if (tooltip.isShowing()) {
@@ -179,7 +179,7 @@ private void addGameCard(Game game) {
 
   private void removeGameCard(Game game) {
     GameTileController gameTileController = uidToGameCard.remove(game.getId());
-    gameTileController.stopGameStatusTimeUpdater();
+    gameTileController.sever();
     Node card = gameTileController.getRoot();
 
     if (card != null) {

diff --git a/src/main/java/com/faforever/client/vault/VaultController.java b/src/main/java/com/faforever/client/vault/VaultController.java
@@ -66,13 +66,13 @@ public void initialize() {
 
       if (newValue == mapVaultTab) {
         eventBus.post(new OpenMapVaultEvent());
-      } else if (newValue == modVaultTab) {
+      } /*else if (newValue == modVaultTab) {
         eventBus.post(new OpenModVaultEvent());
       } else if (newValue == onlineReplayVaultTab) {
         eventBus.post(new OpenOnlineReplayVaultEvent());
       } else if (newValue == localReplayVaultTab) {
         eventBus.post(new OpenReplayVaultEvent());
-      }
+      }*/
       // TODO implement other tabs
     });
   }
@@ -85,7 +85,7 @@ protected void onDisplay(NavigateEvent navigateEvent) {
       if (navigateEvent instanceof OpenMapVaultEvent) {
         lastTab = mapVaultTab;
         lastTabController = mapVaultController;
-      } else if (navigateEvent instanceof OpenModVaultEvent) {
+      } /*else if (navigateEvent instanceof OpenModVaultEvent) {
         lastTab = modVaultTab;
         lastTabController = modVaultController;
       } else if (navigateEvent instanceof OpenOnlineReplayVaultEvent) {
@@ -94,7 +94,7 @@ protected void onDisplay(NavigateEvent navigateEvent) {
       } else if (navigateEvent instanceof OpenReplayVaultEvent) {
         lastTab = localReplayVaultTab;
         lastTabController = localReplayVaultController;
-      }
+      } */
       vaultRoot.getSelectionModel().select(lastTab);
       lastTabController.display(navigateEvent);
     } finally {