Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify recursion constraints between invoke and call #1714

Merged
merged 1 commit into from
Jan 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
59 changes: 59 additions & 0 deletions Jint.Tests/Runtime/ExecutionConstraintTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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>();
Expand Down
34 changes: 29 additions & 5 deletions Jint/Engine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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>
Expand All @@ -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;
}
Expand Down Expand Up @@ -1487,7 +1511,7 @@ internal JsValue Call(FunctionInstance functionInstance, JsValue thisObject)
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);

Expand Down Expand Up @@ -1520,7 +1544,7 @@ internal JsValue Call(FunctionInstance functionInstance, JsValue thisObject)
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);

Expand Down