Unify recursion constraints between invoke and call (#1714)

sebastienros · Jan 1, 2024 · 3dcae81 · 3dcae81
1 parent 1636506
commit 3dcae81
Show file tree

Hide file tree

Showing 2 changed files with 88 additions and 5 deletions.
diff --git a/Jint.Tests/Runtime/ExecutionConstraintTests.cs b/Jint.Tests/Runtime/ExecutionConstraintTests.cs
@@ -299,6 +299,65 @@ public void ShouldConsiderConstraintsWhenCallingInvoke()
             }
         }
 
+        [Fact]
+        public void ResetConstraints()
+        {
+            void ExecuteAction(Engine engine) => engine.Execute("recursion()");
+            void InvokeAction(Engine engine) => engine.Invoke("recursion");
+
+            List<int> expected = [6, 6, 6, 6, 6];
+            Assert.Equal(expected, RunLoop(CreateEngine(), ExecuteAction));
+            Assert.Equal(expected, RunLoop(CreateEngine(), InvokeAction));
+
+            var e1 = CreateEngine();
+            Assert.Equal(expected, RunLoop(e1, ExecuteAction));
+            Assert.Equal(expected, RunLoop(e1, InvokeAction));
+
+            var e2 = CreateEngine();
+            Assert.Equal(expected, RunLoop(e2, InvokeAction));
+            Assert.Equal(expected, RunLoop(e2, ExecuteAction));
+
+            var e3 = CreateEngine();
+            Assert.Equal(expected, RunLoop(e3, InvokeAction));
+            Assert.Equal(expected, RunLoop(e3, ExecuteAction));
+            Assert.Equal(expected, RunLoop(e3, InvokeAction));
+
+            var e4 = CreateEngine();
+            Assert.Equal(expected, RunLoop(e4, InvokeAction));
+            Assert.Equal(expected, RunLoop(e4, InvokeAction));
+        }
+
+        private static Engine CreateEngine()
+        {
+            Engine engine = new(options => options.LimitRecursion(5));
+            return engine.Execute("""
+                  var num = 0;
+                  function recursion() {
+                      num++;
+                      recursion(num);
+                  }
+              """);
+        }
+
+        private static List<int> RunLoop(Engine engine, Action<Engine> engineAction)
+        {
+            List<int> results = new();
+            for (var i = 0; i < 5; i++)
+            {
+                try
+                {
+                    engine.SetValue("num", 0);
+                    engineAction.Invoke(engine);
+                }
+                catch (RecursionDepthOverflowException)
+                {
+                    results.Add((int) engine.GetValue("num").AsNumber());
+                }
+            }
+
+            return results;
+        }
+
         private class MyApi
         {
             public readonly Dictionary<string, ScriptFunctionInstance> Callbacks = new Dictionary<string, ScriptFunctionInstance>();

diff --git a/Jint/Engine.cs b/Jint/Engine.cs
@@ -717,7 +717,7 @@ internal void PutValue(Reference reference, JsValue value)
         /// <returns>The value returned by the function call.</returns>
         public JsValue Invoke(string propertyName, params object?[] arguments)
         {
-            return Invoke(propertyName, null, arguments);
+            return Invoke(propertyName, thisObj: null, arguments);
         }
 
         /// <summary>
@@ -742,7 +742,7 @@ public JsValue Invoke(string propertyName, object? thisObj, object?[] arguments)
         /// <returns>The value returned by the function call.</returns>
         public JsValue Invoke(JsValue value, params object?[] arguments)
         {
-            return Invoke(value, null, arguments);
+            return Invoke(value, thisObj: null, arguments);
         }
 
         /// <summary>
@@ -768,7 +768,31 @@ JsValue DoInvoke()
                     items[i] = JsValue.FromObject(this, arguments[i]);
                 }
 
-                var result = callable.Call(JsValue.FromObject(this, thisObj), items);
+                // ensure logic is in sync between Call, Construct, engine.Invoke and JintCallExpression!
+                JsValue result;
+                var thisObject = JsValue.FromObject(this, thisObj);
+                if (callable is FunctionInstance functionInstance)
+                {
+                    var callStack = CallStack;
+                    callStack.Push(functionInstance, expression: null, ExecutionContext);
+                    try
+                    {
+                        result = functionInstance.Call(thisObject, items);
+                    }
+                    finally
+                    {
+                        // if call stack was reset due to recursive call to engine or similar, we might not have it anymore
+                        if (callStack.Count > 0)
+                        {
+                            callStack.Pop();
+                        }
+                    }
+                }
+                else
+                {
+                    result = callable.Call(thisObject, items);
+                }
+
                 _jsValueArrayPool.ReturnArray(items);
                 return result;
             }
@@ -1487,7 +1511,7 @@ internal JsValue Call(
             JsValue[] arguments,
             JintExpression? expression)
         {
-            // ensure logic is in sync between Call, Construct and JintCallExpression!
+            // ensure logic is in sync between Call, Construct, engine.Invoke and JintCallExpression!
 
             var recursionDepth = CallStack.Push(functionInstance, expression, ExecutionContext);
 
@@ -1520,7 +1544,7 @@ private ObjectInstance Construct(
             JsValue newTarget,
             JintExpression? expression)
         {
-            // ensure logic is in sync between Call, Construct and JintCallExpression!
+            // ensure logic is in sync between Call, Construct, engine.Invoke and JintCallExpression!
 
             var recursionDepth = CallStack.Push(functionInstance, expression, ExecutionContext);