Add StackGuard for improved stack handling support (#1566)

Co-authored-by: Umut Akkaya <umut.akkaya@bilin.com>
Co-authored-by: Marko Lahma <marko.lahma@gmail.com>

Jint.Tests/Runtime/EngineLimitTests.cs

@@ -1,11 +1,19 @@
 #if !NETFRAMEWORK
 
 using System.Text;
+using Jint.Runtime;
 
 namespace Jint.Tests.Runtime;
 
 public class EngineLimitTests
 {
+
+#if RELEASE
+    const int FunctionNestingCount = 960;
+#else
+    const int FunctionNestingCount = 520;
+#endif
+
     [Fact]
     public void ShouldAllowReasonableCallStackDepth()
     {
@@ -15,24 +23,61 @@ public void ShouldAllowReasonableCallStackDepth()
             return;
         }
 
-#if RELEASE
-        const int FunctionNestingCount = 960;
-#else
-        const int FunctionNestingCount = 520;
-#endif
+        var script = GenerateCallTree(FunctionNestingCount);
 
-        // generate call tree
+        var engine = new Engine();
+        engine.Execute(script);
+        Assert.Equal(123, engine.Evaluate("func1(123);").AsNumber());
+        Assert.Equal(FunctionNestingCount, engine.Evaluate("x").AsNumber());
+    }
+
+    [Fact]
+    public void ShouldNotStackoverflowWhenStackGuardEnable()
+    {
+        // Can be more than actual dotnet stacktrace count, It does not hit stackoverflow anymore.
+        int functionNestingCount = FunctionNestingCount * 2;
+
+        var script = GenerateCallTree(functionNestingCount);
+
+        var engine = new Engine(option => option.Constraints.MaxExecutionStackCount = functionNestingCount);
+        engine.Execute(script);
+        Assert.Equal(123, engine.Evaluate("func1(123);").AsNumber());
+        Assert.Equal(functionNestingCount, engine.Evaluate("x").AsNumber());
+    }
+
+    [Fact]
+    public void ShouldThrowJavascriptExceptionWhenStackGuardExceed()
+    {
+        // Can be more than actual dotnet stacktrace count, It does not hit stackoverflow anymore.
+        int functionNestingCount = FunctionNestingCount * 2;
+
+        var script = GenerateCallTree(functionNestingCount);
+
+        var engine = new Engine(option => option.Constraints.MaxExecutionStackCount = 500);
+        try
+        {
+            engine.Execute(script);
+            engine.Evaluate("func1(123);");
+        }
+        catch (JavaScriptException jsException)
+        {
+            Assert.Equal("Maximum call stack size exceeded", jsException.Message);
+        }
+    }
+
+    private string GenerateCallTree(int functionNestingCount)
+    {
         var sb = new StringBuilder();
-        sb.AppendLine("var x = 10;");
+        sb.AppendLine("var x = 1;");
         sb.AppendLine();
-        for (var i = 1; i <= FunctionNestingCount; ++i)
+        for (var i = 1; i <= functionNestingCount; ++i)
         {
             sb.Append("function func").Append(i).Append("(func").Append(i).AppendLine("Param) {");
             sb.Append("    ");
-            if (i != FunctionNestingCount)
+            if (i != functionNestingCount)
             {
                 // just to create a bit more nesting add some constructs
-                sb.Append("return x++ > 1 ? func").Append(i + 1).Append("(func").Append(i).AppendLine("Param): undefined;");
+                sb.Append("return x++ >= 1 ? func").Append(i + 1).Append("(func").Append(i).AppendLine("Param): undefined;");
             }
             else
             {
@@ -43,10 +88,7 @@ public void ShouldAllowReasonableCallStackDepth()
             sb.AppendLine("}");
             sb.AppendLine();
         }
-
-        var engine = new Engine();
-        engine.Execute(sb.ToString());
-        Assert.Equal(123, engine.Evaluate("func1(123);").AsNumber());
+        return sb.ToString();
     }
 }
 

Jint.Tests/Runtime/EngineTests.cs

@@ -1883,6 +1883,31 @@ public void DateShouldParseToString()
             ");
         }
 
+
+        [Fact]
+        public void ShouldThrowErrorWhenMaxExecutionStackCountLimitExceeded()
+        {
+            new Engine(options => options.Constraints.MaxExecutionStackCount = 1000)
+                            .SetValue("assert", new Action<bool>(Assert.True))
+                            .Evaluate(@"
+                    var count = 0;
+                    function recurse() {
+                        count++;
+                        recurse();
+                        return null; // ensure no tail recursion
+                    }
+                    try {
+                        count = 0; 
+                        recurse();
+                        assert(false);
+                    } catch(err) {
+                        assert(count >= 1000);
+                    }
+            ");
+
+        }
+
+
         [Fact]
         public void LocaleNumberShouldUseLocalCulture()
         {

Jint/Engine.cs

@@ -63,6 +63,7 @@ public sealed partial class Engine : IDisposable
         internal ConditionalWeakTable<object, ObjectInstance>? _objectWrapperCache;
 
         internal readonly JintCallStack CallStack;
+        internal readonly StackGuard _stackGuard;
 
         // needed in initial engine setup, for example CLR function construction
         internal Intrinsics _originalIntrinsics = null!;
@@ -129,6 +130,7 @@ public Engine(Action<Engine, Options> options)
             Options.Apply(this);
 
             CallStack = new JintCallStack(Options.Constraints.MaxRecursionDepth >= 0);
+            _stackGuard = new StackGuard(this);
         }
 
         private void Reset()

Jint/Options.cs

@@ -9,6 +9,7 @@
 using Jint.Runtime.Debugger;
 using Jint.Runtime.Descriptors;
 using Jint.Runtime.Modules;
+using Jint.Runtime.CallStack;
 
 namespace Jint
 {
@@ -385,6 +386,16 @@ public class ConstraintOptions
         /// </summary>
         public int MaxRecursionDepth { get; set; } = -1;
 
+        /// <summary>
+        /// Maximum recursion stack count, defaults to -1 (as-is dotnet stacktrace).
+        /// </summary>
+        /// <remarks>
+        /// Chrome and V8 based engines (ClearScript) that can handle 13955.
+        /// When set to a different value except -1, it can reduce slight performance/stack trace readability drawback. (after hitting the engine's own limit),
+        /// When max stack size to be exceeded, Engine throws an exception <see cref="JavaScriptException">.
+        /// </remarks>
+        public int MaxExecutionStackCount { get; set; } = StackGuard.Disabled;
+
         /// <summary>
         /// Maximum time a Regex is allowed to run, defaults to 10 seconds.
         /// </summary>

Jint/Runtime/CallStack/StackGuard.cs

@@ -0,0 +1,87 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+// https://github.com/dotnet/runtime/blob/a0964f9e3793cb36cc01d66c14a61e89ada5e7da/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/StackGuard.cs
+
+using System.Runtime.CompilerServices;
+using System.Threading;
+
+namespace Jint.Runtime.CallStack;
+
+internal sealed class StackGuard
+{
+    public const int Disabled = -1;
+
+    private readonly Engine _engine;
+
+    public StackGuard(Engine engine)
+    {
+        _engine = engine;
+    }
+
+    public bool TryEnterOnCurrentStack()
+    {
+        if (_engine.Options.Constraints.MaxExecutionStackCount == Disabled)
+        {
+            return true;
+        }
+
+#if NETFRAMEWORK || NETSTANDARD2_0
+        try
+        {
+            RuntimeHelpers.EnsureSufficientExecutionStack();
+            return true;
+        }
+        catch (InsufficientExecutionStackException)
+        {
+        }
+#else
+        if (RuntimeHelpers.TryEnsureSufficientExecutionStack())
+        {
+            return true;
+        }
+#endif
+
+        if (_engine.CallStack.Count > _engine.Options.Constraints.MaxExecutionStackCount)
+        {
+            ExceptionHelper.ThrowRangeError(_engine.Realm, "Maximum call stack size exceeded");
+        }
+
+        return false;
+    }
+
+    public TR RunOnEmptyStack<T1, TR>(Func<T1, TR> action, T1 arg1)
+    {
+#if NETFRAMEWORK || NETSTANDARD2_0
+        return RunOnEmptyStackCore(static s =>
+        {
+            var t = (Tuple<Func<T1, TR>, T1>) s;
+            return t.Item1(t.Item2);
+        }, Tuple.Create(action, arg1));
+#else
+        // Prefer ValueTuple when available to reduce dependencies on Tuple
+        return RunOnEmptyStackCore(static s =>
+        {
+            var t = ((Func<T1, TR>, T1)) s;
+            return t.Item1(t.Item2);
+        }, (action, arg1));
+#endif
+
+    }
+
+    private R RunOnEmptyStackCore<R>(Func<object, R> action, object state)
+    {
+        // Using default scheduler rather than picking up the current scheduler.
+        Task<R> task = Task.Factory.StartNew((Func<object?, R>) action, state, CancellationToken.None, TaskCreationOptions.DenyChildAttach, TaskScheduler.Default);
+
+        // Avoid AsyncWaitHandle lazy allocation of ManualResetEvent in the rare case we finish quickly.
+        if (!task.IsCompleted)
+        {
+            // Task.Wait has the potential of inlining the task's execution on the current thread; avoid this.
+            ((IAsyncResult) task).AsyncWaitHandle.WaitOne();
+        }
+
+        // Using awaiter here to propagate original exception
+        return task.GetAwaiter().GetResult();
+    }
+}

Jint/Runtime/Interpreter/Expressions/JintCallExpression.cs

@@ -78,6 +78,11 @@ static bool CanSpread(Node? e)
 
         protected override object EvaluateInternal(EvaluationContext context)
         {
+            if (!context.Engine._stackGuard.TryEnterOnCurrentStack())
+            {
+                return context.Engine._stackGuard.RunOnEmptyStack(EvaluateInternal, context);
+            }
+
             if (_calleeExpression._expression.Type == Nodes.Super)
             {
                 return SuperCall(context);