Offer a stack-overflow guard that turns a process kill into a RangeError - #3005
Merged
Conversation
Unbounded recursion in a default engine ends the host process with a native stack overflow: uncatchable, no JavaScript error, no diagnostics, nothing in the log but an exit code. Eighteen distinct routes into a function body reproduce it, and configuring MaxExecutionStackCount covers one of them, because the only stack probe in the engine lives in JintCallExpression and `new`, an accessor, a valueOf coercion, a Proxy trap and a host Invoke all reach a function body without ever evaluating a call expression. Options.Constraints.StackOverflowGuard puts a probe where every route converges -- ScriptFunction's two [[Call]] entry points, its register-argument entry point and its [[Construct]]. It measures the remaining native stack rather than counting calls, so it adapts to the thread the engine is on instead of to a frame count the host had to guess, and it throws a JavaScriptException, which the interpreter turns into a throw Completion at each level instead of rethrowing. That distinction is what makes the reserve sufficient: measured on a 1.5 MB stack a JavaScriptException unwinds 1153 interpreter frames where a rethrown CLR exception manages 158. It ships opt-in. The rule fixed before the benchmark gate ran was that shipping it on by default needed no call row worse than about 1%. The gate measured the recursion rows -- Fib, DeepSum, Tak -- 1.7-2.3% slower with it on, agreeing across two order-rotated BDN pairs and corroborated by a five-repetition alternating REPL driver at +0.9% median on deep recursion, with hot shallow calls unchanged. Above the line, so the host asks for it: a couple of percent on deep recursion buys a process that survives script it did not write, and a host whose scripts are all its own can bound them with LimitRecursion and pay nothing. The placement stays unconditional even so, because it is what makes the capability reachable at all, and it is cheap to leave there: only the branch on the cached flag inlines into the caller, with the probe and the throw both out of line, so an engine that did not opt in pays one predictable test of a bool field per entry into a script function. Which entry points those are matters now that proper tail calls exist. A strict tail call replaces the caller's frame and runs on ContinueTailCalls' trampoline, so it grows no native stack and there is nothing to probe for; putting the probe in CallOnce or CallCore, which the trampoline re-enters from a loop, would charge a tail recursion per hop for a stack that never moves. It sits on the four entries that do add a frame instead, which leaves sloppy-mode recursion, calls out of tail position and the non-call routes above in scope and leaves proper tail calls alone. MaxExecutionStackCount keeps its own behaviour and takes precedence when a host sets both. It continues the call chain on a fresh thread, and the guard, sitting a few frames deeper, would reach the condition first and leave that lane nothing to hop with. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The Security section lists what a host has to opt into; the guard is one more entry there, and the substance belongs beside the other execution constraints -- what turning it on costs, when that is the trade you want, that a proper tail call needs none of it, and how it composes with LimitRecursion, which counts frames and fires first, and with MaxExecutionStackCount, which is checked at call expressions only and takes precedence over it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
An unbounded recursion in script kills the host process, and it has eighteen doors. The release review reproduced the crash — process exit
0xC00000FD, nocatch, no exception, nothing logged — through every route into a function body it tried: plain calls,new, getters and setters,valueOf/toStringcoercions, Proxy traps, sort comparers,toJSON, class field initializers,superchains, bound functions,Reflect.apply, array callbacks, host delegate re-entry. The engine's existingMaxExecutionStackCountmachinery probes only the call-expression route, so it saw one of the eighteen; every .NET engine except YantraJS crashes the same way by default.This adds
Options.Constraints.StackOverflowGuard— a stack-headroom probe (RuntimeHelpers.TryEnsureSufficientExecutionStack) at the four frame-adding entries into an interpreted function (Call,CallWithStackFrame,CallFromRegisters,Construct), throwing a catchableRangeError: Maximum call stack size exceededat a constant ~83-frame (~115 KB) reserve. The engine survives: call stack empty, pools intact, later evaluations correct — pinned by a recovery battery, including three successive guard hits on one engine firing at the identical depth.It ships opt-in, and the reason is a measurement, not a mood. The decision rule fixed before the gate ran was: default-on needs no call row worse than about 1%. The gate measured the recursion rows (
Fib,DeepSum,Tak) at +1.7–2.3% with the guard on, agreeing across two order-rotated BDN pairs, corroborated by a 5-rep alternating REPL driver at +0.9% median on deep recursion (hot shallow calls ~0%). Above the line → opt-in, with the option's XML doc quoting the numbers so a host can make the same trade knowingly. A host running untrusted input almost certainly wants it on; a host running its own scripts can useMaxRecursionDepthand pay nothing. What inlines into each entry when the option is off is a field load and a never-taken branch to a cold no-inline probe.Proper tail calls (#2975) are exempt, and must be. A strict tail call is re-dispatched by the trampoline without re-entering the probed entries and consumes no native stack — so the probe deliberately does not sit in
CallOnce/CallCore, where it would charge a strict tail recursion one probe per hop for a stack that never moves. Post-PTC the guard's scope is sloppy-mode recursion, strict calls out of tail position, and the sixteen non-call routes. Two tests pin exactly this boundary, and the eight route theories are deliberately sloppy-mode with a doc comment saying why.Relationship to the existing options, documented and pinned:
MaxRecursionDepthcounts frames and fires first;MaxExecutionStackCountselects the older thread-hop lane and takes precedence when both are set — the hop needs the whole callee evaluation re-dispatched onto a fresh thread, which is only possible at the call expression, so the two lanes cannot compose and the guard's wider coverage is reachable throughStackOverflowGuardonly.Out-of-process evidence (per-scenario exits, depth bisection):
LimitRecursion(200)/(1000)and unguarded recursion on 1 MB and 1.5 MB stacks — before:0xC00000FD; guarded: clean exit with the catchable error. Guard depths: 664 of 747 crash-depth frames on 1 MB, 12,094 of 12,177 on 16 MB — the reserve is constant, so it costs proportionally less the more stack a host provisions.33 tests across
Jint.TestsandJint.Tests.PublicInterface, including the default-off pin (ADefaultEngineDoesNotProbeAtAll, measured on a 1 MB thread and verified on a 16 MB one, since the other direction's failure mode is ending the process). Full solution green both TFMs; verification legs green; test262 standalone 99,779 / 0 / 122 — byte-identical to the post-#2975 baseline, whose 35 un-parked tail-call tests all pass.🤖 Generated with Claude Code