diff --git a/src/Lua/Runtime/LuaVirtualMachine.Debug.cs b/src/Lua/Runtime/LuaVirtualMachine.Debug.cs index 777f89be..6e55ef4b 100644 --- a/src/Lua/Runtime/LuaVirtualMachine.Debug.cs +++ b/src/Lua/Runtime/LuaVirtualMachine.Debug.cs @@ -87,16 +87,16 @@ static async ValueTask Impl(VirtualMachineExecutionContext context) stack.Push("line"); stack.Push(line); context.State.IsInHook = true; - var frame = context.State.CreateCallStackFrame(hook, 2, top, pc); - context.State.PushCallStackFrame(frame); - LuaFunctionExecutionContext funcContext = new() - { - State = context.State, - ArgumentCount = stack.Count - frame.Base, - ReturnFrameBase = frame.ReturnBase, - }; try { + var frame = context.State.CreateCallStackFrame(hook, 2, top, pc); + context.State.PushCallStackFrame(frame); + LuaFunctionExecutionContext funcContext = new() + { + State = context.State, + ArgumentCount = stack.Count - frame.Base, + ReturnFrameBase = frame.ReturnBase, + }; await hook.Func(funcContext, context.CancellationToken); } finally @@ -157,22 +157,28 @@ internal static async ValueTask ExecuteCallHook( stack.Push(LuaValue.Nil); context.State.IsInHook = true; - var frame = context.State.CreateCallStackFrame(hook, 2, top, 0); - context.State.PushCallStackFrame(frame); - LuaFunctionExecutionContext funcContext = new() - { - State = context.State, - ArgumentCount = stack.Count - frame.Base, - ReturnFrameBase = frame.ReturnBase, - }; + var framePushed = false; try { + var frame = context.State.CreateCallStackFrame(hook, 2, top, 0); + context.State.PushCallStackFrame(frame); + framePushed = true; + LuaFunctionExecutionContext funcContext = new() + { + State = context.State, + ArgumentCount = stack.Count - frame.Base, + ReturnFrameBase = frame.ReturnBase, + }; await hook.Func(funcContext, cancellationToken); } finally { context.State.IsInHook = false; - context.State.PopCallStackFrameWithStackPop(); + // A failed push leaves the caller's frame on top; do not pop it. + if (framePushed) + { + context.State.PopCallStackFrameWithStackPop(); + } } } @@ -200,23 +206,27 @@ internal static async ValueTask ExecuteCallHook( stack.Push("return"); stack.Push(LuaValue.Nil); context.State.IsInHook = true; - frame = context.State.CreateCallStackFrame(hook, 2, top, 0); - context.State.PushCallStackFrame(frame); - LuaFunctionExecutionContext funcContext = new() - { - State = context.State, - ArgumentCount = stack.Count - frame.Base, - ReturnFrameBase = frame.ReturnBase, - }; + var framePushed = false; try { - context.State.IsInHook = true; + frame = context.State.CreateCallStackFrame(hook, 2, top, 0); + context.State.PushCallStackFrame(frame); + framePushed = true; + LuaFunctionExecutionContext funcContext = new() + { + State = context.State, + ArgumentCount = stack.Count - frame.Base, + ReturnFrameBase = frame.ReturnBase, + }; await hook.Func(funcContext, cancellationToken); } finally { context.State.IsInHook = false; - context.State.PopCallStackFrameWithStackPop(); + if (framePushed) + { + context.State.PopCallStackFrameWithStackPop(); + } } return r; diff --git a/src/Lua/Standard/DebugLibrary.cs b/src/Lua/Standard/DebugLibrary.cs index f0d59b4b..3c927bd1 100644 --- a/src/Lua/Standard/DebugLibrary.cs +++ b/src/Lua/Standard/DebugLibrary.cs @@ -480,29 +480,35 @@ CancellationToken cancellationToken return 0; } - if (state.IsReturnHookEnabled && context.State == state) + // Re-arming a hook must not dispatch another hook while its callback is running. + if (state.IsReturnHookEnabled && context.State == state && !state.IsInHook) { var stack = state.Stack; var top = stack.Count; stack.Push("return"); stack.Push(LuaValue.Nil); context.State.IsInHook = true; - var frame = context.State.CreateCallStackFrame(hook, 2, top, 0); - context.State.PushCallStackFrame(frame); - LuaFunctionExecutionContext funcContext = new() - { - State = context.State, - ArgumentCount = stack.Count - frame.Base, - ReturnFrameBase = frame.ReturnBase, - }; + var framePushed = false; try { + var frame = context.State.CreateCallStackFrame(hook, 2, top, 0); + context.State.PushCallStackFrame(frame); + framePushed = true; + LuaFunctionExecutionContext funcContext = new() + { + State = context.State, + ArgumentCount = stack.Count - frame.Base, + ReturnFrameBase = frame.ReturnBase, + }; await hook.Func(funcContext, cancellationToken); } finally { context.State.IsInHook = false; - context.State.PopCallStackFrameWithStackPop(); + if (framePushed) + { + context.State.PopCallStackFrameWithStackPop(); + } } } diff --git a/tests/Lua.Tests/HookTests.cs b/tests/Lua.Tests/HookTests.cs index aca35aa4..300f8879 100644 --- a/tests/Lua.Tests/HookTests.cs +++ b/tests/Lua.Tests/HookTests.cs @@ -1,30 +1,39 @@ +using Lua.Standard; + namespace Lua.Tests; public class HookTests { const string Loop = "local x=0 for i=1,100 do x=x+i end return x"; - [TestCase(false, "runtime")] - [TestCase(true, "runtime")] - [TestCase(false, "host")] - [TestCase(true, "host")] - [TestCase(false, "cancel")] - [TestCase(true, "cancel")] - public async Task CountHook_AfterException_StateCanRunHooksAgain( - bool asynchronous, - string failure + [Test] + [Combinatorial] + public async Task Hook_AfterException_StateCanRunHooksAgain( + [Values(false, true)] bool asynchronous, + [Values("runtime", "host", "cancel")] string failure, + [Values("count", "line", "call", "return", "sethook-return")] string hookEvent ) { using var state = LuaState.Create(); using var cancellation = new CancellationTokenSource(); var gate = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - state.SetHook( - new("abort", (c, ct) => asynchronous ? ThrowAsync(c, ct) : Throw(c, ct)), - "", - 4 + var hook = new LuaFunction( + "abort", + (c, ct) => asynchronous ? ThrowAsync(c, ct) : Throw(c, ct) ); + var source = Loop; + if (hookEvent == "sethook-return") + { + state.OpenDebugLibrary(); + state.Environment["abort"] = hook; + source = "debug.sethook(abort, 'r') " + Loop; + } + else + { + state.SetHook(hook, GetMask(hookEvent), hookEvent == "count" ? 4 : 0); + } - var execution = state.DoStringAsync(Loop, "hook.lua", cancellation.Token).AsTask(); + var execution = state.DoStringAsync(source, "hook.lua", cancellation.Token).AsTask(); if (asynchronous) { Assert.That(execution.IsCompleted, Is.False); @@ -33,7 +42,18 @@ string failure if (failure == "cancel") { - Assert.ThrowsAsync(async () => await execution); + if (hookEvent is "call" or "return") + { + Assert.ThrowsAsync(async () => await execution); + } + else + { + Assert.ThrowsAsync(async () => await execution); + } + } + else if (failure == "host" && hookEvent is "call" or "return") + { + Assert.ThrowsAsync(async () => await execution); } else { @@ -96,13 +116,51 @@ async ValueTask ThrowAsync(LuaFunctionExecutionContext context, Cancellatio } } - [Test] - public async Task CountHook_WhenFrameLimitIsExceeded_StateCanRunHooksAgain() + [TestCase("count", 1)] + [TestCase("line", 1)] + [TestCase("call", 1)] + [TestCase("return", 1)] + [TestCase("sethook-return", 2)] + public async Task Hook_WhenFrameLimitIsExceeded_StateCanRunHooksAgain( + string hookEvent, + int depth + ) { using var state = LuaState.Create(); - state.MaxCallDepth = 1; - state.SetHook(new((c, ct) => new(c.Return())), "", 4); - Assert.ThrowsAsync(async () => await state.DoStringAsync(Loop)); + state.MaxCallDepth = depth; + var abortedFires = 0; + var hook = new LuaFunction( + (c, ct) => + { + abortedFires++; + return new(c.Return()); + } + ); + var source = Loop; + if (hookEvent == "sethook-return") + { + state.OpenDebugLibrary(); + state.Environment["abort"] = hook; + source = "debug.sethook(abort, 'r') " + Loop; + } + else + { + state.SetHook(hook, GetMask(hookEvent), hookEvent == "count" ? 4 : 0); + } + + if (hookEvent is "call" or "return") + { + Assert.ThrowsAsync(async () => + await state.DoStringAsync(source) + ); + } + else + { + Assert.ThrowsAsync(async () => await state.DoStringAsync(source)); + } + Assert.That(abortedFires, Is.Zero); + Assert.That(state.CallStackFrameCount, Is.Zero); + var stackTop = state.Stack.Count; state.MaxCallDepth = 10; var fires = 0; @@ -120,6 +178,73 @@ public async Task CountHook_WhenFrameLimitIsExceeded_StateCanRunHooksAgain() var result = await state.DoStringAsync(Loop); Assert.That(result[0], Is.EqualTo((LuaValue)5050)); Assert.That(fires, Is.GreaterThan(0)); + Assert.That(state.CallStackFrameCount, Is.Zero); + Assert.That(state.Stack.Count, Is.EqualTo(stackTop)); + } + + static string GetMask(string hookEvent) => + hookEvent switch + { + "line" => "l", + "call" => "c", + "return" => "r", + _ => "", + }; + + [TestCase(false)] + [TestCase(true)] + public async Task DebugSetHook_InsideHook_DoesNotRecursivelyFireReturnHook(bool asynchronous) + { + using var state = LuaState.Create(); + state.OpenDebugLibrary(); + var gate = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var replacementFires = 0; + state.Environment["replacement"] = new LuaFunction( + (c, ct) => + { + replacementFires++; + return new(c.Return()); + } + ); + state.Environment["echo"] = new LuaFunction((c, ct) => new(c.Return(c.GetArgument(0)))); + var originalFires = 0; + state.SetHook( + new( + async (c, ct) => + { + originalFires++; + if (asynchronous) + { + await gate.Task; + } + var result = await state.DoStringAsync( + "debug.sethook(replacement, 'r') return 7", + "rearm.lua", + ct + ); + Assert.That(result[0], Is.EqualTo((LuaValue)7)); + Assert.That(replacementFires, Is.Zero); + return c.Return(); + } + ), + "", + 4 + ); + + var execution = state + .DoStringAsync("local x=0 for i=1,100 do x=x+i end echo(x) return x") + .AsTask(); + if (asynchronous) + { + Assert.That(execution.IsCompleted, Is.False); + gate.SetResult(); + } + var result = await execution; + Assert.That(result[0], Is.EqualTo((LuaValue)5050)); + Assert.That(originalFires, Is.EqualTo(1)); + Assert.That(replacementFires, Is.GreaterThan(0)); + Assert.That(state.CallStackFrameCount, Is.Zero); + Assert.That(state.Stack.Count, Is.Zero); } [TestCase(false)]