Repository navigation
Add proactive stack overflow detection via MaxCallDepth + tests - #317
Merged
akeit0 merged 3 commits intoOct 10, 2026
Merged
Conversation
Contributor
Author
|
And solves #168. |
Contributor
Author
|
@nuskey8 Mind reviewing this? |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds configurable, proactive stack overflow protection to the Lua runtime to avoid process-terminating StackOverflowException and support sandboxing by enforcing a maximum Lua call depth and surfacing a catchable LuaStackOverflowException.
Changes:
- Introduces
LuaState.MaxCallDepthand enforces it when pushing call stack frames. - Adds
RuntimeHelpers.TryEnsureSufficientExecutionStack()checks in VMCALL/TAILCALL. - Makes
LuaStackOverflowExceptionpublic and adds test coverage for max call depth behavior (includingpcall).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Lua.Tests/StackOverflowTests.cs | Adds tests validating max call depth enforcement and pcall behavior. |
| src/Lua/Standard/BasicLibrary.cs | Updates pcall exception handling to return a stack overflow message. |
| src/Lua/Runtime/LuaVirtualMachine.cs | Adds stack-availability checks on CALL and TAILCALL. |
| src/Lua/LuaState.cs | Adds MaxCallDepth and enforces it in PushCallStackFrame. |
| src/Lua/Exceptions.cs | Makes LuaStackOverflowException public for user catchability. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+370
to
+371
| case LuaStackOverflowException: | ||
| return context.Return(false, ex.Message); |
Comment on lines
+1422
to
+1425
| if (!RuntimeHelpers.TryEnsureSufficientExecutionStack()) | ||
| { | ||
| throw new LuaStackOverflowException(); | ||
| } |
Comment on lines
+1628
to
+1631
| if (!RuntimeHelpers.TryEnsureSufficientExecutionStack()) | ||
| { | ||
| throw new LuaStackOverflowException(); | ||
| } |
Comment on lines
+200
to
+201
| public int MaxCallDepth { get; set; } = 100_000; | ||
|
|
Collaborator
|
I would appreciate it if you could measure the performance changes using recursive call benchmarks. |
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Detect excessive call depth and insufficient native stack before invoking LuaFunction delegates, including direct C# RunAsync reentry, hooks and metamethods. The original guards on VM CALL/TAILCALL also ran for pure Lua calls, which stay inside the VM loop, while direct C# reentry had no guard.
Validation on .NET 8 x64 Windows: 304 runtime tests (excluding ExpectedFailure), one source-generator test, and Release builds for net10.0/net8.0/net6.0/netstandard2.1 passed. Tests cover real non-tail recursion, tail-call depth, shared coroutine policy, nil errors, and monitored 1 MiB native-stack reentry through both direct C# and Lua calls. Missing guards fail the test without deliberately overflowing the process stack.
Performance comparison against main 991c4fc with BenchmarkDotNet 0.14.0, two launches, five warmups, twelve iterations, and 500 ms iterations. Scripts are compiled in setup and results are validated. These four call-heavy workloads are not a general application benchmark:
The initial revised C# run was unstable (2.8–5.0 ms); the separate repeat was stable at 2.756 ± 0.0155 ms (99.9% confidence interval). Native-stack protection has a measurable C# call cost. This small cost provides protection for direct C# reentry that main did not check, while ordinary Lua calls remain approximately unchanged and tail recursion improves.
GitHub Ubuntu CI lint and runtime tests also passed on b1453e3 (run 38039314105). The native-stack regression test allows the callback's own frame to cross the runtime's reserve threshold once; the next guarded invocation must throw, and a second low-stack entry stops the test safely if protection is missing.