Keep counting a frame a tail call replaced while its trampoline runs

[lahma]

Options.LimitRecursion stops firing altogether when a proper tail call is on the recursion path, and the host process dies on a native stack overflow that no catch can see. A regression from 4.15.3, introduced by #2975.

Related to #3012/#3017 by theme only, and independent of them in code: there the limit fires and the exception overflows the stack on the way out; here the limit cannot fire at all.

What breaks

MaxRecursionDepth is not a stack-depth limit. JintCallStack._statistics is a multiset of the live stack keyed by JintFunctionDefinition identity (CallStackElementComparer), and every push compares that function's occurrence count against the limit. #3020's program is a two-function cycle:

• a strict getter (one stable definition) ends in return calc() — a proper tail call;
• calc is re-parsed by engine.Execute on every pass, so every pass is a fresh definition (Engine._functionDefinitions is keyed on the AST Node);
• calc's body reads entity.calc, re-entering the getter through Engine.Call — not a tail call, so this genuinely grows the native stack, one cycle at a time, forever.

Through 4.15.3 the getter's frame stayed under calc, the stack read [getter, calc₁, getter, calc₂, …], the getter's count grew 0,1,2,… and the limit fired at 21. From 4.16.0 the tail call replaces the getter's frame, and ReplaceTop removed the displaced function from the statistics:

if (_statistics[previous] == 0) _statistics.Remove(previous);   // the getter drops out entirely
else _statistics[previous]--;

so the stack becomes [calc₁, calc₂, …, calcₙ, getter] — every calcᵢ a distinct definition counted once, and the getter re-added at depth 0 on every cycle because the previous ReplaceTop erased it. Nothing ever reaches 21. CallStack.Count grows without bound while every counter stays at zero.

The compensating bookkeeping inside ContinueTailCalls (depth0/depth1/tailDepths) does count tail hops, but it is local to one activation of that loop. A target that re-enters the engine by a non-tail route lands in a nested trampoline, which re-seeds from GetRecursionDepth — which the erasure has just reset to zero.

The fix

A displaced activation whose trampoline is still running still holds native stack: ContinueTailCalls and everything beneath it have not returned. So ReplaceTop now retains that occurrence and returns the target's depth exactly as Push does; ContinueTailCalls reads the depth from there and hands the retentions back through ReleaseTailRetention in the finally it already had. Retaining is also what the option already promised — "repeated tail transfers are still included in this limit so an infinite strict tail-recursive function terminates with RecursionDepthOverflowException" — the statistic simply did not agree with the trampoline's private counters.

The ledger is tallied against the function each ReplaceTop displaced, not the one it installed: the replacement is accounted by the frame it now names, so it is the displaced occurrence that outlives its element. Two fields carry direct and mutual tail recursion without allocating; anything wider spills to a dictionary. The release is tolerant of a missing entry, because a host callback can call ResetCallStack() from inside a running trampoline.

The shadow counters are gone, and with them GetRecursionDepth/GetNextRecursionDepth, which had no other callers.

Not in scope, now documented

The limit counts occurrences of one function definition, so a recursion whose every level is a function created for that level — eval, new Function, a host re-running a script — repeats no definition and is outside the limit however deep it goes. That predates proper tail calls and was equally true in 4.15.3; Options.Constraints.StackOverflowGuard (#3005) is the only thing that has ever covered it. Both LimitRecursion and StackOverflowGuard say so now — the latter's remarks previously listed only two cases where the probe is the one that answers, and this is a third.

Pre-fix behaviour of the new tests

On main the two regression tests do not fail, they take the runner with them — -1073741571 is 0xC00000FD, STATUS_STACK_OVERFLOW:

[xUnit.net 00:01:00.99]     [FATAL ERROR] Xunit.Sdk.TestPipelineException
[xUnit.net 00:01:00.99] Catastrophic failure: Test process crashed with exit code -1073741571.
No test matches the given testcase filter `FullyQualifiedName~HostTailCallTests.RecursionLimitStillFiresWhenATailCallIsOnThePath` in ...\Jint.Tests.PublicInterface\release_net10.0\Jint.Tests.PublicInterface.dll

[xUnit.net 00:01:01.92]     [FATAL ERROR] Xunit.Sdk.TestPipelineException
[xUnit.net 00:01:01.92] Catastrophic failure: Test process crashed with exit code -1073741571.
No test matches the given testcase filter `FullyQualifiedName~TailCallOptimizationTests.RecursionLimitFiresWhenATailCallReEntersThroughAGetter` in ...\Jint.Tests\release_net10.0\Jint.Tests.dll

Neither needs a large stack afterwards: the limit fires at depth 21, as asked.

• HostTailCallTests.RecursionLimitStillFiresWhenATailCallIsOnThePath is the issue's shape as an embedder writes it — a host delegate that re-runs the script. In Jint.Tests.PublicInterface, which has no InternalsVisibleTo, so it also proves the shape is reachable by a third party.
• TailCallOptimizationTests.RecursionLimitFiresWhenATailCallReEntersThroughAGetter is the same mechanism with no host code at all — indirect eval supplies the per-level function — so this is script-reachable, not an artifact of the reporter's harness. The eval source varies per level on purpose: EvalFunction caches parses by source text, and a cached parse would be one definition, would be counted, and would stop the recursion for the wrong reason.

Two further tests guard the release half, and both pass on main as well as here — they are the net for the new mechanism, not repros:

• HostTailCallTests.CompletedTailDelegationDoesNotAccumulateAgainstTheLimit — 100 iterations of one completed tail delegation under LimitRecursion(0). Without the release this fails on the second iteration.
• TailCallOptimizationTests.RecursionLimitFailureLeavesTheDepthStatisticBalanced — a leaked retention is invisible in CallStack.Count and shows up only later, as a second run of the same functions overflowing before it has recursed at all.

InfiniteStrictTailRecursionHonorsRecursionLimit, MultiFunctionTailCycleHonorsRecursionLimit (the three-definition path, i.e. the dictionary), RecursionLimitFailureLeavesCallStackBalanced and DistinctTailDelegationDoesNotCountAsRecursion are unchanged and still pin the firing depths and the balance.

Benchmarks

Not run, and deliberately. Both changed methods are entered only on a proper tail call, which requires strict mode; SunSpider and Dromaeo are sloppy throughout, so the gate's workloads never reach the diff. The one strict script in Jint.Benchmark/Scripts is handlebars (template-rendering), and for a no-limit engine — which is every benchmark — the new code is strictly less work per hop: ReplaceTop drops a CallStackElement copy and a CallStackElementComparer.Equals call and returns early on null statistics, against one added int compare. The per-trampoline cost is three null checks in a finally. Happy to add a template-rendering before/after row if you would rather see one.

Verification

All Release, all green:

suite	net10.0	net472
Jint.Tests	5725 passed, 4 skipped	5640 passed, 4 skipped
Jint.Tests (JINT_HOST_CONTRACT_VERIFICATION=1)	5724 passed, 4 skipped	5639 passed, 4 skipped
Jint.Tests.PublicInterface	1488 passed, 9 skipped	1487 passed, 9 skipped
Jint.Tests.PublicInterface (JINT_HOST_CONTRACT_VERIFICATION=1)	1492 passed, 5 skipped	1491 passed, 5 skipped
Jint.Tests.Test262	99779 passed, 122 skipped, 0 failed	n/a (net10.0 only)
Jint.Tests.CommonScripts	28 passed	28 passed

Closes #3020

🤖 Generated with Claude Code

https://claude.ai/code/session_0163Srj3aNzScH1keGb9smyg