From a8b2187d939d84cdd0cb25e4349c0a9860b85b8e Mon Sep 17 00:00:00 2001 From: Pavlos Touboulidis Date: Mon, 19 May 2014 02:06:56 +0300 Subject: [PATCH] Possible fix for lua crash #5269 Check if this instance has been disposed and don't call 'tick' if it has. Also remove the finalizer that was broken and wrong anyway. 'runtime' would never become null because it's readonly and managed resources are freed automatically or may have already been freed by then. --- OpenRA.Game/Scripting/ScriptContext.cs | 23 ++++++++++----- OpenRA.Mods.RA/Scripting/CallLuaFunc.cs | 23 ++++++++++----- OpenRA.Mods.RA/Scripting/ScriptTriggers.cs | 33 ++++++++++++++++------ 3 files changed, 56 insertions(+), 23 deletions(-) diff --git a/OpenRA.Game/Scripting/ScriptContext.cs b/OpenRA.Game/Scripting/ScriptContext.cs index dc8c269476..c3bde02cc6 100644 --- a/OpenRA.Game/Scripting/ScriptContext.cs +++ b/OpenRA.Game/Scripting/ScriptContext.cs @@ -81,11 +81,12 @@ namespace OpenRA.Scripting public ScriptGlobalAttribute(string name) { Name = name; } } - public class ScriptContext + public class ScriptContext : IDisposable { public World World { get; private set; } public WorldRenderer WorldRenderer { get; private set; } + bool disposed; readonly MemoryConstrainedLuaRuntime runtime; readonly LuaFunction tick; @@ -188,26 +189,34 @@ namespace OpenRA.Scripting public void Tick(Actor self) { - if (error) + if (error || disposed) return; using (new PerfSample("tick_lua")) tick.Call().Dispose(); } - public void Dispose() + protected void Dispose(bool disposing) { - if (runtime == null) + if (disposed) return; + if (disposing) + runtime.Dispose(); + + disposed = true; + } + + public void Dispose() + { + Dispose(true); GC.SuppressFinalize(this); - runtime.Dispose(); } ~ScriptContext() { - if (runtime != null) - Game.RunAfterTick(Dispose); + // Dispose unmanaged resources only + Dispose(false); } static Type[] ExtractRequiredTypes(Type t) diff --git a/OpenRA.Mods.RA/Scripting/CallLuaFunc.cs b/OpenRA.Mods.RA/Scripting/CallLuaFunc.cs index 276652cce1..7df12f1c30 100644 --- a/OpenRA.Mods.RA/Scripting/CallLuaFunc.cs +++ b/OpenRA.Mods.RA/Scripting/CallLuaFunc.cs @@ -15,12 +15,13 @@ using OpenRA.Traits; namespace OpenRA.Mods.RA.Activities { - public class CallLuaFunc : Activity + public class CallLuaFunc : Activity, IDisposable { LuaFunction function; + public CallLuaFunc(LuaFunction func) { - function = func.CopyReference() as LuaFunction; + function = (LuaFunction)func.CopyReference(); } public override Activity Tick(Actor self) @@ -38,20 +39,28 @@ namespace OpenRA.Mods.RA.Activities base.Cancel(self); } - public void Dispose() + protected void Dispose(bool disposing) { if (function == null) return; + if (disposing) + { + function.Dispose(); + function = null; + } + } + + public void Dispose() + { + Dispose(true); GC.SuppressFinalize(this); - function.Dispose(); - function = null; } ~CallLuaFunc() { - if (function != null) - Game.RunAfterTick(Dispose); + // Dispose unmanaged resources only + Dispose(false); } } } diff --git a/OpenRA.Mods.RA/Scripting/ScriptTriggers.cs b/OpenRA.Mods.RA/Scripting/ScriptTriggers.cs index 1dfe5d0e7a..7056795510 100644 --- a/OpenRA.Mods.RA/Scripting/ScriptTriggers.cs +++ b/OpenRA.Mods.RA/Scripting/ScriptTriggers.cs @@ -21,7 +21,7 @@ namespace OpenRA.Mods.RA.Scripting [Desc("Allows map scripts to attach triggers to this actor via the Triggers global.")] public class ScriptTriggersInfo : TraitInfo { } - public class ScriptTriggers : INotifyIdle, INotifyDamage, INotifyKilled, INotifyProduction + public class ScriptTriggers : INotifyIdle, INotifyDamage, INotifyKilled, INotifyProduction, IDisposable { public event Action OnKilledInternal = _ => {}; @@ -101,21 +101,36 @@ namespace OpenRA.Mods.RA.Scripting } bool disposed; + + protected void Dispose(bool disposing) + { + if (disposed) + return; + + if (disposing) + { + var toDispose = new [] { onIdle, onDamaged, onKilled, onProduction }; + + foreach (var f in toDispose.SelectMany(f => f)) + f.First.Dispose(); + + foreach (var l in toDispose) + l.Clear(); + } + + disposed = true; + } + public void Dispose() { - disposed = true; - var toDispose = new [] { onIdle, onDamaged, onKilled, onProduction }; - - foreach (var f in toDispose.SelectMany(f => f)) - f.First.Dispose(); - + Dispose(true); GC.SuppressFinalize(this); } ~ScriptTriggers() { - if (!disposed) - Game.RunAfterTick(Dispose); + // Dispose unmanaged resources only + Dispose(false); } } }