From 7c889c5ef0da5948f7be526f0f41d48017b2951c Mon Sep 17 00:00:00 2001 From: RoosterDragon Date: Sat, 24 Oct 2015 19:27:05 +0100 Subject: [PATCH] Fix TOCTOU issues when calling Game.RunAfterDelay. Since the action runs after a delay, the state of the game may no longer be the same and it may no longer be valid to run the action. Anything that references the world now calls IsCurrentWorld to ensure the world hasn't changed or been disposed. --- OpenRA.Game/Game.cs | 2 +- .../Traits/Player/ConquestVictoryConditions.cs | 18 +++++++++--------- .../Traits/Player/MissionObjectives.cs | 3 +++ .../Player/StrategicVictoryConditions.cs | 18 +++++++++--------- .../Widgets/Logic/Ingame/IngameMenuLogic.cs | 9 ++++++++- .../LoadIngamePlayerOrObserverUILogic.cs | 3 +++ OpenRA.Mods.D2k/Activities/SwallowActor.cs | 5 +++-- 7 files changed, 36 insertions(+), 22 deletions(-) diff --git a/OpenRA.Game/Game.cs b/OpenRA.Game/Game.cs index c68c711e2e..fae6837950 100644 --- a/OpenRA.Game/Game.cs +++ b/OpenRA.Game/Game.cs @@ -767,7 +767,7 @@ namespace OpenRA public static bool IsCurrentWorld(World world) { - return OrderManager != null && OrderManager.World == world; + return OrderManager != null && OrderManager.World == world && !world.Disposing; } } } diff --git a/OpenRA.Mods.Common/Traits/Player/ConquestVictoryConditions.cs b/OpenRA.Mods.Common/Traits/Player/ConquestVictoryConditions.cs index db7955b785..92a0251fe2 100644 --- a/OpenRA.Mods.Common/Traits/Player/ConquestVictoryConditions.cs +++ b/OpenRA.Mods.Common/Traits/Player/ConquestVictoryConditions.cs @@ -62,22 +62,22 @@ namespace OpenRA.Mods.Common.Traits foreach (var a in player.World.Actors.Where(a => a.Owner == player)) a.Kill(a); - if (player == player.World.LocalPlayer) + Game.RunAfterDelay(info.NotificationDelay, () => { - Game.RunAfterDelay(info.NotificationDelay, () => - { - if (Game.IsCurrentWorld(player.World)) - Game.Sound.PlayNotification(player.World.Map.Rules, player, "Speech", "Lose", player.Faction.InternalName); - }); - } + if (Game.IsCurrentWorld(player.World) && player == player.World.LocalPlayer) + Game.Sound.PlayNotification(player.World.Map.Rules, player, "Speech", "Lose", player.Faction.InternalName); + }); } public void OnPlayerWon(Player player) { Game.Debug("{0} is victorious.", player.PlayerName); - if (player == player.World.LocalPlayer) - Game.RunAfterDelay(info.NotificationDelay, () => Game.Sound.PlayNotification(player.World.Map.Rules, player, "Speech", "Win", player.Faction.InternalName)); + Game.RunAfterDelay(info.NotificationDelay, () => + { + if (Game.IsCurrentWorld(player.World) && player == player.World.LocalPlayer) + Game.Sound.PlayNotification(player.World.Map.Rules, player, "Speech", "Win", player.Faction.InternalName); + }); } public void OnObjectiveAdded(Player player, int id) { } diff --git a/OpenRA.Mods.Common/Traits/Player/MissionObjectives.cs b/OpenRA.Mods.Common/Traits/Player/MissionObjectives.cs index 1e13cd0eff..8f3a5514b4 100644 --- a/OpenRA.Mods.Common/Traits/Player/MissionObjectives.cs +++ b/OpenRA.Mods.Common/Traits/Player/MissionObjectives.cs @@ -150,6 +150,9 @@ namespace OpenRA.Mods.Common.Traits if (gameOver) Game.RunAfterDelay(Info.GameOverDelay, () => { + if (!Game.IsCurrentWorld(player.World)) + return; + player.World.EndGame(); player.World.SetPauseState(true); player.World.PauseStateLocked = true; diff --git a/OpenRA.Mods.Common/Traits/Player/StrategicVictoryConditions.cs b/OpenRA.Mods.Common/Traits/Player/StrategicVictoryConditions.cs index bc45753feb..ffd123d7f8 100644 --- a/OpenRA.Mods.Common/Traits/Player/StrategicVictoryConditions.cs +++ b/OpenRA.Mods.Common/Traits/Player/StrategicVictoryConditions.cs @@ -107,22 +107,22 @@ namespace OpenRA.Mods.Common.Traits foreach (var a in player.World.Actors.Where(a => a.Owner == player)) a.Kill(a); - if (player == player.World.LocalPlayer) + Game.RunAfterDelay(info.NotificationDelay, () => { - Game.RunAfterDelay(info.NotificationDelay, () => - { - if (Game.IsCurrentWorld(player.World)) - Game.Sound.PlayNotification(player.World.Map.Rules, player, "Speech", "Lose", player.Faction.InternalName); - }); - } + if (Game.IsCurrentWorld(player.World) && player == player.World.LocalPlayer) + Game.Sound.PlayNotification(player.World.Map.Rules, player, "Speech", "Lose", player.Faction.InternalName); + }); } public void OnPlayerWon(Player player) { Game.Debug("{0} is victorious.", player.PlayerName); - if (player == player.World.LocalPlayer) - Game.RunAfterDelay(info.NotificationDelay, () => Game.Sound.PlayNotification(player.World.Map.Rules, player, "Speech", "Win", player.Faction.InternalName)); + Game.RunAfterDelay(info.NotificationDelay, () => + { + if (Game.IsCurrentWorld(player.World) && player == player.World.LocalPlayer) + Game.Sound.PlayNotification(player.World.Map.Rules, player, "Speech", "Win", player.Faction.InternalName); + }); } public void OnObjectiveAdded(Player player, int id) { } diff --git a/OpenRA.Mods.Common/Widgets/Logic/Ingame/IngameMenuLogic.cs b/OpenRA.Mods.Common/Widgets/Logic/Ingame/IngameMenuLogic.cs index f749f3d68a..81ff05725b 100644 --- a/OpenRA.Mods.Common/Widgets/Logic/Ingame/IngameMenuLogic.cs +++ b/OpenRA.Mods.Common/Widgets/Logic/Ingame/IngameMenuLogic.cs @@ -51,12 +51,19 @@ namespace OpenRA.Mods.Common.Widgets.Logic var exitDelay = iop != null ? iop.ExitDelay : 0; if (mpe != null) { - Game.RunAfterDelay(exitDelay, () => mpe.Fade(MenuPaletteEffect.EffectType.Black)); + Game.RunAfterDelay(exitDelay, () => + { + if (Game.IsCurrentWorld(world)) + mpe.Fade(MenuPaletteEffect.EffectType.Black); + }); exitDelay += 40 * mpe.Info.FadeLength; } Game.RunAfterDelay(exitDelay, () => { + if (!Game.IsCurrentWorld(world)) + return; + Game.Disconnect(); Ui.ResetAll(); Game.LoadShellMap(); diff --git a/OpenRA.Mods.Common/Widgets/Logic/Ingame/LoadIngamePlayerOrObserverUILogic.cs b/OpenRA.Mods.Common/Widgets/Logic/Ingame/LoadIngamePlayerOrObserverUILogic.cs index f9feaad3e5..976570dd8e 100644 --- a/OpenRA.Mods.Common/Widgets/Logic/Ingame/LoadIngamePlayerOrObserverUILogic.cs +++ b/OpenRA.Mods.Common/Widgets/Logic/Ingame/LoadIngamePlayerOrObserverUILogic.cs @@ -42,6 +42,9 @@ namespace OpenRA.Mods.Common.Widgets.Logic loadingObserverWidgets = true; Game.RunAfterDelay(objectives != null ? objectives.GameOverDelay : 0, () => { + if (!Game.IsCurrentWorld(world)) + return; + playerRoot.RemoveChildren(); Game.LoadWidget(world, "OBSERVER_WIDGETS", playerRoot, new WidgetArgs()); }); diff --git a/OpenRA.Mods.D2k/Activities/SwallowActor.cs b/OpenRA.Mods.D2k/Activities/SwallowActor.cs index 5d7d94de81..e99b5903f0 100644 --- a/OpenRA.Mods.D2k/Activities/SwallowActor.cs +++ b/OpenRA.Mods.D2k/Activities/SwallowActor.cs @@ -109,8 +109,9 @@ namespace OpenRA.Mods.D2k.Activities Game.RunAfterDelay(1000, () => { - foreach (var affectedPlayer in affectedPlayers) - NotifyPlayer(affectedPlayer, attackPosition); + if (Game.IsCurrentWorld(self.World)) + foreach (var affectedPlayer in affectedPlayers) + NotifyPlayer(affectedPlayer, attackPosition); }); }