Bound eval recursion with a catchable RangeError instead of a native stack overflow or an endless thread hop

Jint.Tests.PublicInterface/HostNativeRecursionGuardTests.cs

@@ -530,6 +530,124 @@ public void ADeepTraplessProxyChainReadAsAConstructorRaisesACatchableError(strin
         }, maxStackSize: 1024 * 1024);
     }
 
+    /// <summary>
+    /// Recursions made of evaluations rather than of calls: each level hands source text back to the
+    /// evaluator, and the evaluator re-enters the interpreter without entering a function. The first four
+    /// never reach a function body at all, so none of the probes a function entry carries saw them, and a
+    /// direct eval (<c>var s = 'eval(s)'; eval(s)</c>) ended the host process with a native stack overflow on
+    /// every target framework. The <c>@@hasInstance</c> row is <c>eval</c> reached with no call expression in
+    /// the loop, which nothing else on that route probes either.
+    /// <para>
+    /// The two <c>Function</c> constructor rows were never broken — the function the constructor builds is
+    /// entered through an ordinary call, whose callee probes — and are here so the pair states the shape
+    /// that was: evaluating source text is guarded, whichever built-in hands it over.
+    /// </para>
+    /// </summary>
+    public static TestCases<string, string> EvaluatedSourceRecursions => new()
+    {
+        { "direct eval", "var s = 'eval(s)'; eval(s);" },
+        { "indirect eval", "var s = '(0, eval)(s)'; (0, eval)(s);" },
+        { "optional-call eval", "var s = 'eval?.(s)'; eval?.(s);" },
+        { "eval as a callback", "var s = '[s].forEach(eval)'; [s].forEach(eval);" },
+        { "eval as @@hasInstance", "var o = {}; Object.defineProperty(o, Symbol.hasInstance, { value: eval }); var s = 's instanceof o'; s instanceof o;" },
+        { "new Function", "var s = 'new Function(s)()'; new Function(s)();" },
+        { "Function.prototype.constructor", "var s = 'Function.prototype.constructor(s)()'; Function.prototype.constructor(s)();" },
+    };
+
+    [TestCaseSource(nameof(EvaluatedSourceRecursions))]
+    public void ARecursionThroughEvaluatedSourceRaisesACatchableErrorAndTheEngineRecovers(string route, string script)
+    {
+        _ = route;
+        DedicatedThread.Run(() =>
+        {
+            using var engine = new Engine();
+            EvaluateCatching(engine, script).Should().Be("RangeError:Maximum call stack size exceeded");
+
+            engine.Evaluate("eval('6 * 7')").AsNumber().Should().Be(42);
+        }, maxStackSize: 1024 * 1024);
+    }
+
+    /// <summary>
+    /// The same recursions on the <see cref="Options.ConstraintOptions.MaxExecutionStackCount"/> lane, which
+    /// continues a call chain on a fresh thread when the stack runs low and throws once the call stack holds
+    /// more than the configured count. A direct eval is dispatched without a call-stack frame, so a recursion
+    /// made only of direct evals never grew that count: it hopped to a new thread at every exhaustion, leaving
+    /// the one below it blocked, and never threw — the host lost a thread per hop and the call never returned.
+    /// It counts as the call it is now, so the lane throws at its limit as it does for any other recursion.
+    /// <c>eval?.()</c> took the same frameless dispatch and hung the same way; the other rows push a frame per
+    /// level (the <c>EvalFunction</c>, <c>forEach</c>, the constructed function) and were bounded already.
+    /// <para>
+    /// The join ceiling is what reports the hop that never ends, and is never what passes a test: a bounded
+    /// recursion returns in well under a second. <c>@@hasInstance</c> is not a row: with no call expression in
+    /// its loop it never reaches this lane's check, which is the gap the property's documentation states for
+    /// every route into a function body other than a call.
+    /// </para>
+    /// </summary>
+    public static TestCases<string, string> CountedEvaluatedSourceRecursions => new()
+    {
+        { "direct eval", "var s = 'eval(s)'; eval(s);" },
+        { "indirect eval", "var s = '(0, eval)(s)'; (0, eval)(s);" },
+        { "optional-call eval", "var s = 'eval?.(s)'; eval?.(s);" },
+        { "eval as a callback", "var s = '[s].forEach(eval)'; [s].forEach(eval);" },
+        { "new Function", "var s = 'new Function(s)()'; new Function(s)();" },
+    };
+
+    [TestCaseSource(nameof(CountedEvaluatedSourceRecursions))]
+    public void OnTheExecutionStackCountLaneARecursionThroughEvaluatedSourceStopsAtTheCount(string route, string script)
+    {
+        _ = route;
+        DedicatedThread.Run(
+            () =>
+            {
+                using var engine = new Engine(options => options.Constraints.MaxExecutionStackCount = 500);
+                EvaluateCatching(engine, script).Should().Be("RangeError:Maximum call stack size exceeded");
+
+                engine.Evaluate("eval('6 * 7')").AsNumber().Should().Be(42);
+            },
+            joinTimeout: TestBudgets.WedgeCeiling,
+            timeoutMessage: $"'{route}' did not stop at the configured count within {TestBudgets.WedgeCeiling}; the lane is hopping threads without bound",
+            maxStackSize: 1024 * 1024);
+    }
+
+    /// <summary>
+    /// What the lane is <em>for</em>, through eval: a recursion deeper than the thread holds, but finite and
+    /// inside the configured count, continues on a fresh thread and returns its answer. It is what rules out
+    /// the other way to bound the rows above — probing eval under
+    /// <see cref="Options.ConstraintOptions.StackOverflowGuard"/> on this lane too. That probe sits a few
+    /// frames below the call expression's hop, so whichever of the two finds the stack low first decides, and
+    /// measured on .NET 10 it was the probe: this recursion threw where it used to return, while .NET Framework,
+    /// with other frame sizes, still hopped. Every level counts twice here (the function and the eval), which
+    /// the count leaves ample room for.
+    /// </summary>
+    public static TestCases<string, string> FiniteRecursionsThroughEval => new()
+    {
+        { "direct eval", "function f(n) { return n === 0 ? 0 : eval('f(n - 1)') + 1; } f(3000);" },
+        { "indirect eval", "function f(n) { return n === 0 ? 0 : (0, eval)('f(' + (n - 1) + ')') + 1; } f(3000);" },
+    };
+
+    [TestCaseSource(nameof(FiniteRecursionsThroughEval))]
+    public void OnTheExecutionStackCountLaneAFiniteRecursionThroughEvalDeeperThanTheThreadReturns(string route, string script)
+    {
+        _ = route;
+        DedicatedThread.Run(
+            () =>
+            {
+                using var engine = new Engine(options => options.Constraints.MaxExecutionStackCount = 100_000);
+                engine.Evaluate(script).AsNumber().Should().Be(3000);
+            },
+            joinTimeout: TestBudgets.WedgeCeiling,
+            timeoutMessage: $"'{route}' did not return within {TestBudgets.WedgeCeiling}",
+            maxStackSize: 1024 * 1024);
+    }
+
+    private static string EvaluateCatching(Engine engine, string script) => engine.Evaluate("""
+        var caught;
+        try {
+        """ + script + """
+        } catch (error) { caught = error; }
+        caught === undefined ? 'none' : caught.name + ':' + caught.message;
+        """).AsString();
+
     private sealed class RecursiveHostFunction : HostFunction
     {
         private readonly Engine _hostEngine;

Jint/Constraints/AGENTS.md

@@ -58,7 +58,7 @@ rest of the list is split across the files indexed from the repository-root [`AG
 - **An engine-driven fan-out is one entry, not one per callback — and that is what makes a multi-listener dispatch bounded.** The rule above is about a *host* loop; its mirror image is a loop the engine runs on the host's behalf. `Engine.WebApi.InvokeFetchHandler` firing a `fetch` event at three script-registered listeners brackets `GlobalEventTarget.DispatchEvent` **once** (`Engine.WebApi.Operations.cs`, `DispatchFetchEvent`), so `ResetConstraints()` runs exactly twice — in and out — with all three listeners in between, and they share one `MaxStatements` allowance, one armed `TimeConstraint` deadline, one `MemoryLimitConstraint` segment and one reset of any host-written `Constraint`. The reason is narrower than it looks and worth knowing before touching either side: a listener is invoked through `ICallable.Call` and so never reaches `ExecuteWithConstraints` at all, which means the bracket is not what *prevents* a per-listener re-arm but what *arms* the dispatch in the first place. Remove it and the dispatch runs against whatever the previous entry left — except for allocation, which `Engine.BeginImplicitMemoryOperation` would still bound, but per listener rather than per dispatch. Any new engine-driven fan-out (another event target the engine dispatches at, a batch callback) inherits the decision: bracket the fan-out, never the element. Pinned from the embedder's side in `Jint.Tests.PublicInterface/WebApiFetchHandlerTests.cs`, by tests that were each confirmed to fail against a build with the bracket moved inside the per-listener loop — the single-listener test that predated them passes that build, which is exactly why counting listeners is the only way to pin this.
 - **A public entry that checks constraints without being bracketed does not inherit *no* budget — it inherits the *last* one.** `ExecuteWithConstraints` calls `ResetConstraints()` on the way in and again on the way out for a non-nested entry, so a run *ends* with `TimeConstraint._deadline` armed at `now + interval`. An entry that calls `Constraints.Check()` without going through the bracket therefore measures against a deadline belonging to whatever ran last, however long ago — which is the same mechanism the root [`AGENTS.md`](../../AGENTS.md) records for a host-side `Constraints.Check()`, arriving at a *public API* where no host would look for it. `JsonParser.Parse` was the one instance (sebastienros/jint#3342): the same document on the same engine parsed on a fresh engine, threw `TimeoutException` after an unrelated `Evaluate` and a second's idling, and parsed again from inside a script — because there the surrounding `Evaluate` *was* the bracket. The memory side fails the other way and more quietly: with no entry to attach to, `MemoryLimitConstraint._activeState` is null, `Check()` returns early, and `LimitMemory` bounds nothing. So the rule for any new public entry that observes constraints is that it takes the bracket, and the reason there is exactly one bracket to take — `ExecuteWithConstraints`, whose callback receives its source text as a `ReadOnlySpan<char>` *parameter* rather than closing over it — is that a span cannot be captured by a `Func<T>` and a second copy written for span-taking entries would drift. Pinned from the embedder's side in `Jint.Tests.PublicInterface/HostJsonParseConstraintTests.cs`, including the half a fix breaks: a parse reached from script must go on taking the nested branch, or `JSON.parse` becomes a way for a script to refresh a budget it has already spent.
 - **Cancellation is the one thing that does span host calls, because the amortized check *cadence* is engine state.** It is the sibling of the entry above and resolves the other way. A budget must reset per run; a *cadence* must not. `Engine._amortizedConstraintCountdown` therefore lives on the engine and is deliberately **not** rewound by `ResetConstraints()`, so the statements of one short call carry the countdown forward into the next and detection latency stays bounded at `EvaluationContext.AmortizedConstraintCheckInterval` (64) statements *for the engine*, not for one entry. It used to live on `EvaluationContext` and restart at 64 in the constructor, and since every top-level entry builds a fresh context, a callee shorter than 64 statements never reached an amortized check **at all** — which silently defeated `CancellationConstraint.Reset()` being a no-op, the whole point of which is that cancellation survives across entries. A one-statement predicate invoked after `cts.Cancel()` now throws within 64 calls instead of never. Two consequences worth holding on to: this bounds *latency*, never a budget — `TimeoutInterval` is still re-armed per entry and still does not fire across a host loop — and a nested re-entry shares the countdown rather than restarting it, which is correct for a cadence and is what stops a host callback re-entering the engine from resetting the poll interval on every iteration. Any constraint that declares `IsAmortizable => true` inherits all of this, including a user-derived one.
-- **`MaxRecursionDepth` counts one function's occurrences, not stack depth — and a tail call's displaced frame keeps counting.** `JintCallStack._statistics` is a multiset of the live stack keyed by `JintFunctionDefinition` identity (`CallStackElementComparer`), so the limit asks "how many times is *this* function on the stack", never "how deep is the stack". Two consequences. A recursion whose every level is a function created for that level — `eval`, `new Function`, a host re-running a script through a callback — repeats no definition and is invisible to the limit however deep it goes; `Options.Constraints.StackOverflowGuard` is the only thing that covers that shape, and no version of Jint has covered it any other way. And `JintCallStack.ReplaceTop` deliberately does **not** discount the frame a proper tail call replaced: the frame is gone, but the activation is not over while `ScriptFunction.ContinueTailCalls` is still on the native stack, so the occurrence is retained and handed back through `ReleaseTailRetention` in that trampoline's `finally`. Discounting it (which is what the code did through 4.16.0) made the limit blind to a recursion that leaves and re-enters the trampoline by a non-tail route — a getter, `new`, a coercion, a Proxy trap, a host callback — because every re-entry pushed the displaced function again at depth zero while the native stack kept growing, and `LimitRecursion` then ended in a process-killing stack overflow instead of a `RecursionDepthOverflowException`. Pinned from both sides in `Jint.Tests.PublicInterface/HostTailCallTests.cs` and `Jint.Tests/Runtime/TailCallOptimizationTests.cs`; the release half has its own test, because without it a loop over one *completed* tail delegation accumulates against the limit.
+- **`MaxRecursionDepth` counts one function's occurrences, not stack depth — and a tail call's displaced frame keeps counting.** `JintCallStack._statistics` is a multiset of the live stack keyed by `JintFunctionDefinition` identity (`CallStackElementComparer`), so the limit asks "how many times is *this* function on the stack", never "how deep is the stack". Two consequences. A recursion whose every level is a function created for that level — `eval`, `new Function`, a host re-running a script through a callback — repeats no definition and is invisible to the limit however deep it goes; `Options.Constraints.StackOverflowGuard` is the only thing that covers that shape (`eval` enters no function, so `PerformEval` probes for itself), and no version of Jint has covered it any other way. And `JintCallStack.ReplaceTop` deliberately does **not** discount the frame a proper tail call replaced: the frame is gone, but the activation is not over while `ScriptFunction.ContinueTailCalls` is still on the native stack, so the occurrence is retained and handed back through `ReleaseTailRetention` in that trampoline's `finally`. Discounting it (which is what the code did through 4.16.0) made the limit blind to a recursion that leaves and re-enters the trampoline by a non-tail route — a getter, `new`, a coercion, a Proxy trap, a host callback — because every re-entry pushed the displaced function again at depth zero while the native stack kept growing, and `LimitRecursion` then ended in a process-killing stack overflow instead of a `RecursionDepthOverflowException`. Pinned from both sides in `Jint.Tests.PublicInterface/HostTailCallTests.cs` and `Jint.Tests/Runtime/TailCallOptimizationTests.cs`; the release half has its own test, because without it a loop over one *completed* tail delegation accumulates against the limit.
 - **The native stack probe sits in three kinds of place, none removable, and with the default options it is the only thing between a deep script and a dead process.** `StackOverflowGuard` is on, `MaxRecursionDepth` is `-1` and `MaxExecutionStackCount` disabled for a default engine, so `StackGuard.EnsureNativeStackHeadroom` → `RuntimeHelpers.TryEnsureSufficientExecutionStack` is what turns unbounded recursion into a catchable `RangeError`. Remove it where it is load-bearing and the failure is not a red test but a **native stack overflow no `catch` sees and no assertion survives** — the symptom is a dead test host. The *dispatcher* probe in `Engine.CallNativeFunction` is the only probe for the forty-odd built-in `Function` subclasses whose own `Call` does not probe, a host `Constructor` subclass among them. The *leaf* probes atop `ClrFunction.Call`, `HostFunction.Call`, `DelegateWrapper.Call` and `BindFunction.Call` are the only probes on the ~170 sites that invoke `ICallable.Call` with no dispatcher above them — `JintCallExpression`'s own native branch, `Engine.Invoke`'s copy, `Function.prototype.apply`, every callback dispatch, every bound- and proxy-forwarding hop. **So the asymmetry between the two call dispatchers is deliberate and must not be "fixed":** a dispatcher probe in `JintCallExpression` or `Engine.Invoke` buys nothing the leaf did not already do and costs an FCall on every host method call. Those four leaf probes are pinned by name, read out of the IL rather than a hand-kept list, by `StackOverflowGuardTests.ExactlyTheInteropAndForwardingFunctionsProbeTheNativeStack`, so deleting one fails an assertion naming the type instead of ending the test host — which is what deleting `ClrFunction`'s actually does, inside `Jint.Tests.PublicInterface/HostNativeRecursionGuardTests.cs`. The **third** kind is a *hand-over*: a recursive walk that is otherwise a loop, at the point it stops being one. `ObjectInstance`'s `[[Get]]`/`[[Set]]`/`[[HasProperty]]` walk a prototype chain iteratively and probe only where they hand a link the rest of the algorithm, and `JsProxy` probes on the `return target.<op>(…)` of every internal method, because a trapless proxy forwards its whole algorithm and `new Proxy(new Proxy(…))` is a recursion script controls ([#4076](https://github.com/sebastienros/jint/issues/4076)). **A hand-over probe goes on the forward, never at the operation's entry:** a *trapped* proxy reaches its trap through `ICallable.Call`, where the callee already probes for itself, so an entry probe would be the same redundant third probe this list's previous bullet records as a NO-GO — on the shape real code ships, since a reactivity library traps every object it proxies. Those sites are deliberately **not** in the IL pin above: it scans `Function` subclasses' `Call`, none of these is one, and a probe placed on a rare forwarding hop needs no list to stay honest — the depth tests in `HostNativeRecursionGuardTests` are what fail if one goes, by ending the host.
 - **Nothing probes an exception dispatch, so a recursive native path never throws from inside a `catch`.** A throw in a handler is dispatched on top of every frame down to the throw it handles; one per level turned the probe's `RangeError` into a dead host on its way back up a `ShadowRealm` wrapped-function chain. Record the failure and throw once the handler has returned, as `WrappedFunction.Call` does.
 - **The one route that probes twice is known, deliberate, and measured as not worth resolving — do not re-derive it.** A host accessor read goes `ObjectInstance.UnwrapFromGetter` → `Engine.Call` → `CallNativeFunction` (probe) → `ClrFunction.Call` (probe). Removing one of the two measured as no change on the subject row or any DOM row ([#4013](https://github.com/sebastienros/jint/issues/4013)); the paired table and its calibration caveat are in [`Jint.Benchmark/AGENTS.md`](../../Jint.Benchmark/AGENTS.md#a-measured-no-go-the-double-native-stack-probe).