Fix completion value clobbered by re-entrant Evaluate() during module execution

Jint.Tests/Runtime/EngineTests.cs

@@ -2041,17 +2041,17 @@ public void RegExpPrototypeToString()
         RunTest("assert(RegExp.prototype.toString() === '/(?:)/');");
     }
 
-    [Fact]
-    public void ShouldSetYearBefore1970()
-    {
-        new Engine(options => options.LocalTimeZone(TimeZoneInfo.Utc))
-            .SetValue("equal", new Action<object, object>(Assert.Equal))
-            .Execute(@"
-                var d = new Date('1969-01-01T08:17:00Z');
-                d.setYear(2015);
-                equal('2015-01-01T08:17:00.000Z', d.toISOString());
-            ");
-    }
+    [Fact]
+    public void ShouldSetYearBefore1970()
+    {
+        new Engine(options => options.LocalTimeZone(TimeZoneInfo.Utc))
+            .SetValue("equal", new Action<object, object>(Assert.Equal))
+            .Execute(@"
+                var d = new Date('1969-01-01T08:17:00Z');
+                d.setYear(2015);
+                equal('2015-01-01T08:17:00.000Z', d.toISOString());
+            ");
+    }
 
     [Fact]
     public void ShouldUseReplaceMarkers()
@@ -2873,6 +2873,30 @@ public void RecursiveCallStack()
         Assert.Equal(1389, result);
     }
 
+    [Fact]
+    public void NestedEvaluateDuringContinuationDrainDoesNotClobberOuterResult()
+    {
+        // Regression test for https://github.com/sebastienros/jint/issues/2492
+        // A queued microtask, drained while a nested Evaluate() is running, used to overwrite
+        // the engine-level completion value that the nested Evaluate() was about to return.
+        var engine = new Engine();
+        engine.SetValue("evalB", new Func<JsValue>(() => engine.Evaluate("'B'")));
+        engine.SetValue("evalC", new Func<JsValue>(() => engine.Evaluate("'C'")));
+
+        // Promise.resolve().then(evalC) queues a microtask. evalB()'s nested Evaluate drains it
+        // (RunAvailableContinuations), running evalC() -> a deeper Evaluate that completes with 'C'.
+        // Pre-fix, 'C' leaked into the shared field that evalB()'s Evaluate read back, so the
+        // outer script observed 'C' instead of 'B'.
+        var result = engine.Evaluate(@"
+            var captured;
+            Promise.resolve().then(() => { evalC(); });
+            captured = evalB();
+            captured;
+        ");
+
+        Assert.Equal("B", result.AsString());
+    }
+
     [Fact]
     public void MemberExpressionInObjectProperty()
     {
@@ -2899,43 +2923,43 @@ public void MemberExpressionInObjectProperty()
     }
 
     [Fact]
-    public void TypeofShouldEvaluateOnce()
-    {
-        var engine = new Engine();
-        var result = engine.Evaluate(@"
-                let res = 0;
+    public void TypeofShouldEvaluateOnce()
+    {
+        var engine = new Engine();
+        var result = engine.Evaluate(@"
+                let res = 0;
                 const fn = () => res++;
                 typeof fn();
                 res;
                 ")
             .AsNumber();
-
-        Assert.Equal(1, result);
-    }
-
-    [Fact]
-    public void MemberExpressionOnAssignmentShouldEvaluateRightHandSideOnce()
-    {
-        var engine = new Engine();
-
-        engine.Execute(@"
-            var callCount = 0;
-            function produce() { callCount++; return 'abc'; }
-            var x;
-            var len = (x = produce()).length;
-            ");
-
-        Assert.Equal(1, engine.Evaluate("callCount").AsNumber());
-        Assert.Equal(3, engine.Evaluate("len").AsNumber());
-        Assert.Equal("abc", engine.Evaluate("x").AsString());
-    }
-
-    [Fact]
-    public void ClassDeclarationHoisting()
-    {
-        var ex = Assert.Throws<JavaScriptException>(() => _engine.Evaluate("typeof MyClass; class MyClass {}"));
-        Assert.Equal("Cannot access 'MyClass' before initialization", ex.Message);
-    }
+
+        Assert.Equal(1, result);
+    }
+
+    [Fact]
+    public void MemberExpressionOnAssignmentShouldEvaluateRightHandSideOnce()
+    {
+        var engine = new Engine();
+
+        engine.Execute(@"
+            var callCount = 0;
+            function produce() { callCount++; return 'abc'; }
+            var x;
+            var len = (x = produce()).length;
+            ");
+
+        Assert.Equal(1, engine.Evaluate("callCount").AsNumber());
+        Assert.Equal(3, engine.Evaluate("len").AsNumber());
+        Assert.Equal("abc", engine.Evaluate("x").AsString());
+    }
+
+    [Fact]
+    public void ClassDeclarationHoisting()
+    {
+        var ex = Assert.Throws<JavaScriptException>(() => _engine.Evaluate("typeof MyClass; class MyClass {}"));
+        Assert.Equal("Cannot access 'MyClass' before initialization", ex.Message);
+    }
 
     [Fact]
     public void ShouldObeyScriptLevelStrictModeInFunctions()

Jint.Tests/Runtime/ModuleTests.cs

@@ -121,6 +121,60 @@ public void ShouldImportDynamically()
         Assert.True(received);
     }
 
+    [Fact]
+    public void NestedEvaluateFromModuleExecutionDoesNotClobberCompletionValue()
+    {
+        // Regression test for https://github.com/sebastienros/jint/issues/2492
+        // A CLR function (importLibrary) calls back into engine.Evaluate(). While that nested
+        // Evaluate() drains the event loop, an awaited dynamic import runs module-two, which also
+        // calls importLibrary() -> a deeper Evaluate(). Pre-fix, the deeper completion value
+        // leaked into the field the first importLibrary('library-one') call read back, so it
+        // returned library-two's object and "firstLibrary.TextFunctions" was undefined.
+        var engine = new Engine();
+
+        const string libraryOne = @"(function () {
+            var $export = {};
+            $export.TextFunctions = { generateRandomText: function (length) { return ''; } };
+            return $export;
+        })();";
+
+        const string libraryTwo = @"(function () {
+            var $export = {};
+            $export.DateFunctions = { getCurrentDate: function () { return new Date(); } };
+            return $export;
+        })();";
+
+        engine.SetValue("importLibrary", new Func<string, JsValue>(name => name switch
+        {
+            "library-one" => engine.Evaluate(libraryOne),
+            "library-two" => engine.Evaluate(libraryTwo),
+            _ => JsValue.Undefined
+        }));
+
+        engine.Modules.Add("module-one", "export async function fetchSamplePost() { return new Promise((resolve) => { }); }");
+        engine.Modules.Add("module-two", @"
+            import * as bl from 'module-one';
+            export async function sample() {
+                const lib2 = importLibrary('library-two');
+                globalThis.currentDateFromLib2 = lib2.DateFunctions.getCurrentDate();
+                return await bl.fetchSamplePost();
+            }");
+
+        var result = engine.Evaluate(@"
+            var result = {};
+            (async () => {
+                const m2 = await import('module-two');
+                const obj = await m2.sample();
+                result.obj = obj;
+            })();
+            const firstLibrary = importLibrary('library-one');
+            result.someText = firstLibrary.TextFunctions.generateRandomText(10);
+            result;
+        ").AsObject();
+
+        Assert.Equal("", result.Get("someText").AsString());
+    }
+
     [Fact]
     public void ShouldPropagateParseError()
     {

Jint/Engine.Async.cs

@@ -47,7 +47,7 @@ public Task<JsValue> EvaluateAsync(in Prepared<Script> preparedScript, Cancellat
     /// <returns>The engine instance for chaining, after all async work completes.</returns>
     public async Task<Engine> ExecuteAsync(string code, string? source = null, CancellationToken cancellationToken = default)
     {
-        var result = Execute(code, source)._completionValue;
+        var result = Evaluate(code, source);
         if (result is JsPromise)
         {
             await UnwrapResultAsync(result, cancellationToken).ConfigureAwait(false);

Jint/Engine.cs

@@ -41,7 +41,16 @@ public sealed partial class Engine : IDisposable
     private ParserOptions? _defaultModuleParserOptions; // cache default ParserOptions for ModuleBuilder instances
 
     private readonly ExecutionContextStack _executionContexts;
-    private JsValue _completionValue = JsValue.Undefined;
+
+    // Invariant for engine-level ambient mutable fields (e.g. _activeEvaluationContext, _error,
+    // _lastSyntaxElement): they must not hold an in-flight result across a call that can re-enter
+    // the engine (anything that may run RunAvailableContinuations, or a CLR callback that calls
+    // Evaluate/Execute/Invoke). Either save-and-restore around the re-entrant region (see the
+    // ownsContext pattern in ExecuteWithConstraints and ShadowRealm), or carry the value out-of-band
+    // rather than in a field. A script's completion value used to live here as a field and was
+    // clobbered by a re-entrant drain (https://github.com/sebastienros/jint/issues/2492); it is now
+    // returned from ScriptEvaluation as a per-frame local. _error and _lastSyntaxElement are safe
+    // because they are produced and consumed synchronously with no re-entrant drain in between.
     internal EvaluationContext? _activeEvaluationContext;
     internal ErrorDispatchInfo? _error;
 
@@ -385,7 +394,7 @@ public JsValue Evaluate(string code, string source, ScriptParsingOptions parsing
     /// Evaluates code and returns last return value.
     /// </summary>
     public JsValue Evaluate(in Prepared<Script> preparedScript)
-        => Execute(preparedScript)._completionValue;
+        => ExecuteForCompletion(preparedScript);
 
     /// <summary>
     /// Executes code into engine and returns the engine instance (useful for chaining).
@@ -416,6 +425,17 @@ public Engine Execute(string code, string source, ScriptParsingOptions parsingOp
     /// Executes code into engine and returns the engine instance (useful for chaining).
     /// </summary>
     public Engine Execute(in Prepared<Script> preparedScript)
+    {
+        ExecuteForCompletion(preparedScript);
+        return this;
+    }
+
+    /// <summary>
+    /// Shared core for <see cref="Execute(in Prepared{Script})"/> and <see cref="Evaluate(in Prepared{Script})"/>.
+    /// Returns the script's completion value directly (a per-frame local), so a re-entrant drain
+    /// during <see cref="RunAvailableContinuations"/> cannot clobber it via shared engine state.
+    /// </summary>
+    private JsValue ExecuteForCompletion(in Prepared<Script> preparedScript)
     {
         if (!preparedScript.IsValid)
         {
@@ -425,15 +445,14 @@ public Engine Execute(in Prepared<Script> preparedScript)
         var script = preparedScript.Program;
         var parserOptions = preparedScript.ParserOptions;
         var strict = _isStrict || script.Strict;
-        ExecuteWithConstraints(strict, () => ScriptEvaluation(new ScriptRecord(Realm, script, script.Location.SourceFile), parserOptions));
-
-        return this;
+        // The lambda captures the locals (script, parserOptions), never the `in` parameter.
+        return ExecuteWithConstraints(strict, () => ScriptEvaluation(new ScriptRecord(Realm, script, script.Location.SourceFile), parserOptions));
     }
 
     /// <summary>
     /// https://tc39.es/ecma262/#sec-runtime-semantics-scriptevaluation
     /// </summary>
-    private Engine ScriptEvaluation(ScriptRecord scriptRecord, ParserOptions parserOptions)
+    private JsValue ScriptEvaluation(ScriptRecord scriptRecord, ParserOptions parserOptions)
     {
         Debugger.OnBeforeEvaluate(scriptRecord.EcmaScriptCode);
 
@@ -474,12 +493,17 @@ private Engine ScriptEvaluation(ScriptRecord scriptRecord, ParserOptions parserO
                 throw ex;
             }
 
-            _completionValue = result.GetValueOrDefault();
+            // Capture into a local BEFORE draining the event loop. RunAvailableContinuations can
+            // re-enter the engine (e.g. an awaited dynamic import that invokes a CLR callback which
+            // calls Evaluate()); a deeper ScriptEvaluation must not be able to clobber this frame's
+            // result. A local is per-frame, so it survives re-entrancy (#2492). A completed script's
+            // value is fixed by its last statement — continuations cannot change it.
+            var completionValue = result.GetValueOrDefault();
 
             // TODO what about callstack and thrown exceptions?
             RunAvailableContinuations();
 
-            return this;
+            return completionValue;
         }
         finally
         {