Attempt to fix memory leak with TrackableTypes.

The problem was that TrackableTypes were all global (held in static variables), but kept around state (objLookup maps) that shouldn't be global - e.g. that could contain per game data. Additionally, it seems this state wasn't always getting cleared correctly and thus would cause leaking of memory over multiple games - which would especially be a problem for Simulated AI code which created temporary games for simulation.

This change attempts to fix the issue by moving the storage of objLookup maps to the Tracker object, which corresponds to a single Game. This way, there is no cross-contamination between different games and the state is properly cleaned up when a Game goes away.

forge-game/src/main/java/forge/trackable/TrackableObject.java

@@ -30,6 +30,10 @@ public abstract class TrackableObject implements IIdentifiable, Serializable {
         return id;
     }
 
+    public final Tracker getTracker() {
+        return tracker;
+    }
+
     @Override
     public int hashCode() {
         return id;

forge-game/src/main/java/forge/trackable/TrackableTypes.java

@@ -37,13 +37,16 @@ public class TrackableTypes {
     }
 
     public static abstract class TrackableObjectType<T extends TrackableObject> extends TrackableType<T> {
-        private final HashMap<Integer, T> objLookup = new HashMap<Integer, T>();
-
         private TrackableObjectType() {
         }
 
+        protected HashMap<Integer, T> getObjLookup(T obj) {
+            return obj.getTracker().getObjLookupForType(this);
+        }
+
         public T lookup(T from) {
             if (from == null) { return null; }
+            HashMap<Integer, T> objLookup = getObjLookup(from);
             T to = objLookup.get(from.getId());
             if (to == null) {
                 objLookup.put(from.getId(), from);
@@ -52,12 +55,9 @@ public class TrackableTypes {
             return to;
         }
 
-        public void clearLookupDictionary() {
-            objLookup.clear();
-        }
-
         @Override
         protected void updateObjLookup(T newObj) {
+            HashMap<Integer, T> objLookup = getObjLookup(newObj);
             if (newObj != null && !objLookup.containsKey(newObj.getId())) {
                 objLookup.put(newObj.getId(), newObj);
                 newObj.updateObjLookup();
@@ -68,6 +68,7 @@ public class TrackableTypes {
         protected void copyChangedProps(TrackableObject from, TrackableObject to, TrackableProperty prop) {
             T newObj = from.get(prop);
             if (newObj != null) {
+                HashMap<Integer, T> objLookup = getObjLookup(newObj);
                 T existingObj = objLookup.get(newObj.getId());
                 if (existingObj != null) { //if object exists already, update its changed properties
                     existingObj.copyChangedProps(newObj);
@@ -107,14 +108,14 @@ public class TrackableTypes {
                 for (int i = 0; i < newCollection.size(); i++) {
                     T newObj = newCollection.get(i);
                     if (newObj != null) {
-                        T existingObj = itemType.objLookup.get(newObj.getId());
+                        T existingObj = itemType.getObjLookup(newObj).get(newObj.getId());
                         if (existingObj != null) { //if object exists already, update its changed properties
                             existingObj.copyChangedProps(newObj);
                             newCollection.remove(i);
                             newCollection.add(i, existingObj);
                         }
                         else { //if object is new, cache in object lookup
-                            itemType.objLookup.put(newObj.getId(), newObj);
+                            itemType.getObjLookup(newObj).put(newObj.getId(), newObj);
                         }
                     }
                 }

forge-game/src/main/java/forge/trackable/Tracker.java

@@ -1,12 +1,16 @@
 package forge.trackable;
 
+import java.util.HashMap;
 import java.util.List;
 
 import com.google.common.collect.Lists;
 
+import forge.trackable.TrackableTypes.TrackableType;
+
 public class Tracker {
     private int freezeCounter = 0;
     private final List<DelayedPropChange> delayedPropChanges = Lists.newArrayList();
+    private final HashMap<TrackableType<?>, Object> objLookups = new HashMap<>();
 
     public final boolean isFrozen() {
         return freezeCounter > 0;
@@ -16,6 +20,18 @@ public class Tracker {
         freezeCounter++;
     }
 
+    // Note: objLookups exist on the tracker and not on the TrackableType because
+    // TrackableType is global and Tracker is per game.
+    public <T> HashMap<Integer, T> getObjLookupForType(TrackableType<T> type) {
+        @SuppressWarnings("unchecked")
+        HashMap<Integer, T> result = (HashMap<Integer, T>) objLookups.get(type);
+        if (result == null) {
+            result = new HashMap<>();
+            objLookups.put(type, result);
+        }
+        return result;
+    }
+
     public void unfreeze() {
         if (!isFrozen() || --freezeCounter > 0 || delayedPropChanges.isEmpty()) {
             return;

forge-gui/src/main/java/forge/match/AbstractGuiGame.java

@@ -72,10 +72,6 @@ public abstract class AbstractGuiGame implements IGuiGame, IMayViewCards {
     public void setGameView(final GameView gameView0) {
         if (gameView == null || gameView0 == null) {
             if (gameView0 != null) {
-                //ensure lookup dictionaries are reset before each game
-                TrackableTypes.CardViewType.clearLookupDictionary();
-                TrackableTypes.PlayerViewType.clearLookupDictionary();
-                TrackableTypes.StackItemViewType.clearLookupDictionary();
                 gameView0.updateObjLookup();
             }
             gameView = gameView0;