Unverified Commit 012d69a7 authored by Stephen Toub's avatar Stephen Toub Committed by GitHub
Browse files

Fix finalizability of Lua instances (#531)

* Fix finalizability of Lua instances

Lua instances end up being permanently rooted until Dispose is explicitly called, because the ctor roots the Lua instance into a static singleton of ObjectTranslatorPool. This fixes it by replacing the ConcurrentDictionary with a ConditionalWeakTable, in order to avoid the pool keeping a strong reference to the LuaState / Lua objects.

* Try an alternate approach

Instead of having the ObjectTranslatorPool store weak references, which means LuaState objects might get collected when still referenced by native code, have the ObjectTranslator store a weak rather than strong reference to the Lua object, which is what has the finalizer.

* Disable test on mono
parent c0539601
...@@ -1056,8 +1056,7 @@ namespace NLua ...@@ -1056,8 +1056,7 @@ namespace NLua
var debug = LuaDebug.FromIntPtr(luaDebug); var debug = LuaDebug.FromIntPtr(luaDebug);
ObjectTranslator translator = ObjectTranslatorPool.Instance.Find(state); ObjectTranslator translator = ObjectTranslatorPool.Instance.Find(state);
Lua lua = translator.Interpreter; translator.Interpreter?.DebugHookCallbackInternal(debug);
lua.DebugHookCallbackInternal(debug);
} }
private void DebugHookCallbackInternal(LuaDebug luaDebug) private void DebugHookCallbackInternal(LuaDebug luaDebug)
......
...@@ -13,7 +13,7 @@ namespace NLua ...@@ -13,7 +13,7 @@ namespace NLua
protected bool TryGet(out Lua lua) protected bool TryGet(out Lua lua)
{ {
if (_lua.State == null) if (_lua?.State == null)
{ {
lua = null; lua = null;
return false; return false;
...@@ -83,4 +83,4 @@ namespace NLua ...@@ -83,4 +83,4 @@ namespace NLua
return _Reference; return _Reference;
} }
} }
} }
\ No newline at end of file
...@@ -1350,9 +1350,10 @@ namespace NLua ...@@ -1350,9 +1350,10 @@ namespace NLua
catch (TargetInvocationException e) catch (TargetInvocationException e)
{ {
// Failure of method invocation // Failure of method invocation
if (_translator.interpreter.UseTraceback) Lua interpreter = _translator.Interpreter;
e.GetBaseException().Data["Traceback"] = _translator.interpreter.GetDebugTraceback(); if (interpreter?.UseTraceback is true)
return _translator.Interpreter.SetPendingException(e.GetBaseException()); e.GetBaseException().Data["Traceback"] = interpreter.GetDebugTraceback();
return interpreter?.SetPendingException(e.GetBaseException()) ?? 0;
} }
catch (Exception e) catch (Exception e)
{ {
......
...@@ -104,7 +104,7 @@ namespace NLua.Method ...@@ -104,7 +104,7 @@ namespace NLua.Method
/// <param name="e">null for no pending exception</param> /// <param name="e">null for no pending exception</param>
int SetPendingException(Exception e) int SetPendingException(Exception e)
{ {
return _translator.interpreter.SetPendingException(e); return _translator.Interpreter?.SetPendingException(e) ?? 0;
} }
void FillMethodArguments(LuaState luaState, int numStackToSkip) void FillMethodArguments(LuaState luaState, int numStackToSkip)
...@@ -175,8 +175,9 @@ namespace NLua.Method ...@@ -175,8 +175,9 @@ namespace NLua.Method
catch (TargetInvocationException e) catch (TargetInvocationException e)
{ {
// Failure of method invocation // Failure of method invocation
if (_translator.interpreter.UseTraceback) Lua interpreter = _translator.Interpreter;
e.GetBaseException().Data["Traceback"] = _translator.interpreter.GetDebugTraceback(); if (interpreter?.UseTraceback is true)
e.GetBaseException().Data["Traceback"] = interpreter.GetDebugTraceback();
return SetPendingException(e.GetBaseException()); return SetPendingException(e.GetBaseException());
} }
catch (Exception e) catch (Exception e)
......
...@@ -58,14 +58,14 @@ namespace NLua ...@@ -58,14 +58,14 @@ namespace NLua
MetaFunctions metaFunctions; MetaFunctions metaFunctions;
List<Assembly> assemblies; List<Assembly> assemblies;
internal CheckType typeChecker; internal CheckType typeChecker;
internal Lua interpreter; private WeakReference<Lua> interpreter;
/// <summary> /// <summary>
/// We want to ensure that objects always have a unique ID /// We want to ensure that objects always have a unique ID
/// </summary> /// </summary>
int _nextObj; int _nextObj;
public MetaFunctions MetaFunctionsInstance => metaFunctions; public MetaFunctions MetaFunctionsInstance => metaFunctions;
public Lua Interpreter => interpreter; public Lua Interpreter => interpreter.TryGetTarget(out Lua lua) ? lua : null;
public IntPtr Tag => _tagPtr; public IntPtr Tag => _tagPtr;
readonly IntPtr _tagPtr; readonly IntPtr _tagPtr;
...@@ -73,7 +73,7 @@ namespace NLua ...@@ -73,7 +73,7 @@ namespace NLua
public ObjectTranslator(Lua interpreter, LuaState luaState) public ObjectTranslator(Lua interpreter, LuaState luaState)
{ {
_tagPtr = Marshal.AllocHGlobal(Marshal.SizeOf(typeof(int))); _tagPtr = Marshal.AllocHGlobal(Marshal.SizeOf(typeof(int)));
this.interpreter = interpreter; this.interpreter = new WeakReference<Lua>(interpreter);
typeChecker = new CheckType(this); typeChecker = new CheckType(this);
metaFunctions = new MetaFunctions(this); metaFunctions = new MetaFunctions(this);
assemblies = new List<Assembly>(); assemblies = new List<Assembly>();
...@@ -220,10 +220,11 @@ namespace NLua ...@@ -220,10 +220,11 @@ namespace NLua
string message = e as string; string message = e as string;
Lua interpreter = Interpreter;
if (message != null) if (message != null)
{ {
// Wrap Lua error (just a string) and store the error location // Wrap Lua error (just a string) and store the error location
if (interpreter.UseTraceback) if (interpreter?.UseTraceback is true)
message += Environment.NewLine + interpreter.GetDebugTraceback(); message += Environment.NewLine + interpreter.GetDebugTraceback();
e = new LuaScriptException(message, errLocation); e = new LuaScriptException(message, errLocation);
} }
...@@ -234,7 +235,7 @@ namespace NLua ...@@ -234,7 +235,7 @@ namespace NLua
if (ex != null) if (ex != null)
{ {
// Wrap generic .NET exception as an InnerException and store the error location // Wrap generic .NET exception as an InnerException and store the error location
if (interpreter.UseTraceback) ex.Data["Traceback"] = interpreter.GetDebugTraceback(); if (interpreter?.UseTraceback is true) ex.Data["Traceback"] = interpreter.GetDebugTraceback();
e = new LuaScriptException(ex, errLocation); e = new LuaScriptException(ex, errLocation);
} }
} }
...@@ -907,7 +908,7 @@ namespace NLua ...@@ -907,7 +908,7 @@ namespace NLua
int reference = luaState.Ref(LuaRegistry.Index); int reference = luaState.Ref(LuaRegistry.Index);
if (reference == -1) if (reference == -1)
return null; return null;
return new LuaTable(reference, interpreter); return new LuaTable(reference, Interpreter);
} }
/* /*
...@@ -922,7 +923,7 @@ namespace NLua ...@@ -922,7 +923,7 @@ namespace NLua
int reference = luaState.Ref(LuaRegistry.Index); int reference = luaState.Ref(LuaRegistry.Index);
if (reference == -1) if (reference == -1)
return null; return null;
return new LuaThread(reference, interpreter); return new LuaThread(reference, Interpreter);
} }
/* /*
...@@ -937,7 +938,7 @@ namespace NLua ...@@ -937,7 +938,7 @@ namespace NLua
int reference = luaState.Ref(LuaRegistry.Index); int reference = luaState.Ref(LuaRegistry.Index);
if (reference == -1) if (reference == -1)
return null; return null;
return new LuaUserData(reference, interpreter); return new LuaUserData(reference, Interpreter);
} }
/* /*
...@@ -952,7 +953,7 @@ namespace NLua ...@@ -952,7 +953,7 @@ namespace NLua
int reference = luaState.Ref(LuaRegistry.Index); int reference = luaState.Ref(LuaRegistry.Index);
if (reference == -1) if (reference == -1)
return null; return null;
return new LuaFunction(reference, interpreter); return new LuaFunction(reference, Interpreter);
} }
/* /*
......
...@@ -16,7 +16,7 @@ namespace NLua ...@@ -16,7 +16,7 @@ namespace NLua
public void Add(LuaState luaState, ObjectTranslator translator) public void Add(LuaState luaState, ObjectTranslator translator)
{ {
if(!translators.TryAdd(luaState, translator)) if (!translators.TryAdd(luaState, translator))
throw new ArgumentException("An item with the same key has already been added. ", "luaState"); throw new ArgumentException("An item with the same key has already been added. ", "luaState");
} }
...@@ -24,7 +24,7 @@ namespace NLua ...@@ -24,7 +24,7 @@ namespace NLua
{ {
ObjectTranslator translator; ObjectTranslator translator;
if(!translators.TryGetValue(luaState, out translator)) if (!translators.TryGetValue(luaState, out translator))
{ {
LuaState main = luaState.MainThread; LuaState main = luaState.MainThread;
...@@ -41,4 +41,3 @@ namespace NLua ...@@ -41,4 +41,3 @@ namespace NLua
} }
} }
} }
...@@ -17,6 +17,7 @@ using LuaFunction = NLua.LuaFunction; ...@@ -17,6 +17,7 @@ using LuaFunction = NLua.LuaFunction;
using System.Diagnostics; using System.Diagnostics;
using System.Collections.Generic; using System.Collections.Generic;
using System.Linq; using System.Linq;
using System.Runtime.CompilerServices;
// ReSharper disable StringLiteralTypo // ReSharper disable StringLiteralTypo
...@@ -226,10 +227,32 @@ namespace NLuaTest ...@@ -226,10 +227,32 @@ namespace NLuaTest
Console.WriteLine("Was using " + startingMem / 1024 / 1024 + "MB, now using: " + endMem / 1024 / 1024 + "MB"); Console.WriteLine("Was using " + startingMem / 1024 / 1024 + "MB, now using: " + endMem / 1024 / 1024 + "MB");
} }
[Test]
[Platform(Exclude = "mono", Reason = "The test requires a precise GC.")]
public void TestFinalize()
{
WeakReference luaRef = CreateWeakReferenceToLuaInstance();
for (int i = 0; i < 3 && luaRef.IsAlive; i++)
{
GC.Collect();
GC.WaitForPendingFinalizers();
}
Assert.False(luaRef.IsAlive);
}
[MethodImpl(MethodImplOptions.NoInlining)]
private WeakReference CreateWeakReferenceToLuaInstance()
{
Lua lua = new Lua();
_Calc(lua, 42);
return new WeakReference(lua);
}
private void _Calc(Lua lua, int i) private void _Calc(Lua lua, int i)
{ {
lua.DoString( lua.DoString(
"sqrt = math.sqrt;" + "sqrt = math.sqrt;" +
"sqr = function(x) return math.pow(x,2); end;" + "sqr = function(x) return math.pow(x,2); end;" +
"log = math.log;" + "log = math.log;" +
"log10 = math.log10;" + "log10 = math.log10;" +
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment