Definition-level environment reuse for Function-constructor instances

Jint.Tests/Runtime/DynamicFunctionTests.cs

@@ -0,0 +1,53 @@
+namespace Jint.Tests.Runtime;
+
+/// <summary>
+/// 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.
+/// </summary>
+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());
+    }
+}

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)
                 {

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();

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
         {

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);