diff --git a/Jint.Tests/Runtime/DynamicFunctionTests.cs b/Jint.Tests/Runtime/DynamicFunctionTests.cs new file mode 100644 index 000000000..a83076df1 --- /dev/null +++ b/Jint.Tests/Runtime/DynamicFunctionTests.cs @@ -0,0 +1,53 @@ +namespace Jint.Tests.Runtime; + +/// +/// Functions created by the Function constructor share a definition-level call environment +/// across their one-shot instances; every call must still observe fresh bindings, and +/// closure-capturing bodies must never share state. +/// +public class DynamicFunctionTests +{ + [Fact] + public void RepeatedDynamicFunctionCallsGetFreshBindings() + { + var engine = new Engine(static options => options.Strict()); + engine.Execute(""" + var results = []; + for (var k = 0; k < 5; k++) { + results.push(new Function("var c = (typeof c === 'undefined') ? 1 : c + 1; return c;")()); + } + """); + + Assert.Equal("1,1,1,1,1", engine.Evaluate("results.join(',')").AsString()); + } + + [Fact] + public void ClosureCapturingDynamicFunctionsKeepIndependentEnvironments() + { + var engine = new Engine(static options => options.Strict()); + engine.Execute(""" + var counters = []; + for (var k = 0; k < 3; k++) { + counters.push(new Function("var x = 0; return function () { return ++x; };")()); + } + var results = [counters[0](), counters[0](), counters[1](), counters[2]()]; + """); + + Assert.Equal("1,2,1,1", engine.Evaluate("results.join(',')").AsString()); + } + + [Fact] + public void InterleavedInstancesOfTheSameSourceStayIndependent() + { + var engine = new Engine(static options => options.Strict()); + engine.Execute(""" + var src = "var n = seed; seed = seed + 1; return n;"; + var seed = 10; + var f1 = new Function(src); + var f2 = new Function(src); + var results = [f1(), f2(), f1()]; + """); + + Assert.Equal("10,11,12", engine.Evaluate("results.join(',')").AsString()); + } +} diff --git a/Jint/Native/Function/FunctionInstance.Dynamic.cs b/Jint/Native/Function/FunctionInstance.Dynamic.cs index dc15c85a7..964d86b17 100644 --- a/Jint/Native/Function/FunctionInstance.Dynamic.cs +++ b/Jint/Native/Function/FunctionInstance.Dynamic.cs @@ -166,7 +166,10 @@ internal Function CreateDynamicFunction( { Parser parser = new(parserOptions); var function = (IFunction) parser.ParseScriptGuarded(callerRealm, functionExpression, strict: _engine._isStrict).Body[0]; - definition = new JintFunctionDefinition(function); + definition = new JintFunctionDefinition(function) + { + IsDynamic = true, + }; if (cacheable) { diff --git a/Jint/Native/Function/ScriptFunction.cs b/Jint/Native/Function/ScriptFunction.cs index 3eb9defca..ea2068b79 100644 --- a/Jint/Native/Function/ScriptFunction.cs +++ b/Jint/Native/Function/ScriptFunction.cs @@ -173,8 +173,20 @@ protected internal override JsValue Call(JsValue thisObject, JsCallArguments arg funcEnv._slots = null; } - // Cache the env itself so the next call to this function avoids the allocation. - _envReuse = funcEnv; + if (_functionDefinition!.IsDynamic) + { + // Function-constructor instances are one-shot (a fresh ScriptFunction per + // `new Function(...)`), so an instance-level cache never warms. Park the env + // on the per-realm definition instead — env identity then stays stable across + // instances, keeping the shared statement tree's per-node slot caches valid. + funcEnv._outerEnv = null; + Interlocked.Exchange(ref state._dynamicCachedEnv, funcEnv); + } + else + { + // Cache the env itself so the next call to this function avoids the allocation. + _envReuse = funcEnv; + } } } _engine.LeaveExecutionContext(); diff --git a/Jint/Runtime/Environments/JintEnvironment.cs b/Jint/Runtime/Environments/JintEnvironment.cs index 2cbed09c9..24234cee2 100644 --- a/Jint/Runtime/Environments/JintEnvironment.cs +++ b/Jint/Runtime/Environments/JintEnvironment.cs @@ -128,12 +128,30 @@ internal static FunctionEnvironment NewFunctionEnvironment(Engine engine, Functi // Only plain calls (newTarget undefined) rent: the construct path never returns its env to the // cache, so renting there would just drain it and starve the next call's reuse. if (state is { EnvironmentMayEscape: false, IsDirectRecursive: false } - && newTarget.IsUndefined() - && (ScriptFunction) f is { _envReuse: { } envReuse } scriptFunction) + && newTarget.IsUndefined()) { - scriptFunction._envReuse = null; - env = (FunctionEnvironment) envReuse; - env.Reset(f, newTarget, f._environment); + if ((ScriptFunction) f is { _envReuse: { } envReuse } scriptFunction) + { + scriptFunction._envReuse = null; + env = (FunctionEnvironment) envReuse; + env.Reset(f, newTarget, f._environment); + } + else if (f._functionDefinition!.IsDynamic + && Interlocked.Exchange(ref state._dynamicCachedEnv, null) is { } dynamicEnv + && ReferenceEquals(dynamicEnv._engine, engine)) + { + // one-shot Function-constructor instances share a definition-level environment; + // see State._dynamicCachedEnv for why this is exempt from the no-envs-on-State rule + env = dynamicEnv; + env.Reset(f, newTarget, f._environment); + } + else + { + env = new FunctionEnvironment(engine, f, newTarget) + { + _outerEnv = f._environment, + }; + } } else { diff --git a/Jint/Runtime/Interpreter/JintFunctionDefinition.cs b/Jint/Runtime/Interpreter/JintFunctionDefinition.cs index 96bbeea71..b74a92c6a 100644 --- a/Jint/Runtime/Interpreter/JintFunctionDefinition.cs +++ b/Jint/Runtime/Interpreter/JintFunctionDefinition.cs @@ -28,6 +28,12 @@ internal sealed class JintFunctionDefinition public readonly string? Name; public readonly IFunction Function; + // True for definitions created by the Function constructor (CreateDynamicFunction). Their + // definition lives in the per-realm dynamic-function cache and every `new Function(...)` + // produces a fresh ScriptFunction instance, which changes where call environments can be + // safely and usefully cached — see State._dynamicCachedEnv. + public bool IsDynamic; + // Stores the AST node needed for creating the source text. // (This might be different from the Function node, e.g., in the case of class methods.) public readonly INode SourceTextNode; @@ -392,6 +398,15 @@ internal sealed class State // Interlocked is required: parallel test fixtures share cached States (see PR #2418 fallout). public Binding[]? _cachedSlots; + // Exception to the "no environments on State" rule above, for Function-constructor + // definitions only (JintFunctionDefinition.IsDynamic): their definition lives in the + // per-realm dynamic-function cache and is never shared across engines, while every + // `new Function(...)` call produces a fresh ScriptFunction whose per-instance cache can + // never warm. Parking the call environment here keeps one stable environment identity + // across those one-shot instances, which is what the shared statement tree's per-node + // slot caches key on. Interlocked for the same reason as _cachedSlots. + public FunctionEnvironment? _dynamicCachedEnv; + public SourceText SourceText; internal readonly record struct VariableValuePair(Key Name, JsValue? InitialValue);