From 0e9b327fd9c2d60e450f1dfa2fdc8781e048aa1f Mon Sep 17 00:00:00 2001
From: Myrd <myrd@cardforge.org>
Date: Thu, 29 Dec 2016 22:31:18 +0000
Subject: [PATCH] [Simulated AI] Fix memory leak caused by SAs making it into
 the wrong game's spabCache.

The problem was that during copying of SAs, the spabCache was not updated correctly due to the code being in the wrong order. A further problem was that activatingPlayer was not being correctly set on the new abilities, so you would end up referencing a Player from a different Game object from an SA that was in a spabCache, resulting in many previously-simulated Game objects being leaked through this reference chain.

This change fixes both of those problems and also adds a validation function that checks the contents of the spabCache at every game copy.
---
 .../main/java/forge/ai/simulation/GameCopier.java   | 13 ++++++++++++-
 forge-game/src/main/java/forge/game/Game.java       | 10 ++++++++++
 .../src/main/java/forge/game/GameEntityCache.java   |  5 +++++
 .../java/forge/game/spellability/SpellAbility.java  |  2 +-
 4 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/forge-ai/src/main/java/forge/ai/simulation/GameCopier.java b/forge-ai/src/main/java/forge/ai/simulation/GameCopier.java
index aa51649a5a4..be82f5ad2cd 100644
--- a/forge-ai/src/main/java/forge/ai/simulation/GameCopier.java
+++ b/forge-ai/src/main/java/forge/ai/simulation/GameCopier.java
@@ -108,7 +108,18 @@ public class GameCopier {
             ((PlayerZoneBattlefield) p.getZone(ZoneType.Battlefield)).setTriggers(true);
         }
         newGame.getTriggerHandler().clearSuppression(TriggerType.ChangesZone);
-        
+
+        for (Card c : newGame.getCardsInGame()) {
+            for (SpellAbility sa : c.getSpellAbilities()) {
+                Player activatingPlayer = sa.getActivatingPlayer();
+                if (activatingPlayer != null && activatingPlayer.getGame() != newGame) {
+                    sa.setActivatingPlayer(gameObjectMap.map(activatingPlayer));
+                }
+            }
+        }
+        origGame.validateSpabCache();
+        newGame.validateSpabCache();
+
         // Undo effects first before calculating them below, to avoid them applying twice.
         for (StaticEffect effect : origGame.getStaticEffects().getEffects()) {
             effect.removeMapped(gameObjectMap);
diff --git a/forge-game/src/main/java/forge/game/Game.java b/forge-game/src/main/java/forge/game/Game.java
index 6b6c548cf12..4efd5bdb017 100644
--- a/forge-game/src/main/java/forge/game/Game.java
+++ b/forge-game/src/main/java/forge/game/Game.java
@@ -199,6 +199,16 @@ public class Game {
     public void removeSpellAbility(SpellAbility spellAbility) {
         spabCache.remove(spellAbility.getId());
     }
+    public void validateSpabCache() {
+        for (SpellAbility sa : spabCache.getValues()) {
+            if (sa.getHostCard() != null && sa.getHostCard().getGame() != this) {
+                throw new RuntimeException();
+            }
+            if (sa.getActivatingPlayer() != null && sa.getActivatingPlayer().getGame() != this) {
+                throw new RuntimeException();
+            }
+        }
+    }
 
     public Game(List<RegisteredPlayer> players0, GameRules rules0, Match match0) { /* no more zones to map here */
         rules = rules0;
diff --git a/forge-game/src/main/java/forge/game/GameEntityCache.java b/forge-game/src/main/java/forge/game/GameEntityCache.java
index 182568341cf..aeb3145f9c8 100644
--- a/forge-game/src/main/java/forge/game/GameEntityCache.java
+++ b/forge-game/src/main/java/forge/game/GameEntityCache.java
@@ -1,6 +1,7 @@
 package forge.game;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
 
@@ -40,4 +41,8 @@ public class GameEntityCache<Entity extends IIdentifiable, View extends Trackabl
     public void clear() {
         entityCache.clear();
     }
+
+    public Collection<Entity> getValues() {
+        return entityCache.values();
+    }
 }
diff --git a/forge-game/src/main/java/forge/game/spellability/SpellAbility.java b/forge-game/src/main/java/forge/game/spellability/SpellAbility.java
index 684dff7092c..e3d6efeb3f9 100644
--- a/forge-game/src/main/java/forge/game/spellability/SpellAbility.java
+++ b/forge-game/src/main/java/forge/game/spellability/SpellAbility.java
@@ -203,9 +203,9 @@ public abstract class SpellAbility extends CardTraitBase implements ISpellAbilit
     @Override
     public void setHostCard(final Card c) {
         if (hostCard == c) { return; }
-        super.setHostCard(c);
         Game oldGame = hostCard != null ? hostCard.getGame() : null;
         Game newGame = c != null ? c.getGame() : null;
+        super.setHostCard(c);
         if (oldGame != newGame) {
             if (oldGame != null) { oldGame.removeSpellAbility(this); }
             if (newGame != null) { newGame.addSpellAbility(this); }