diff --git a/AGENTS.md b/AGENTS.md index 32451bae6c..c7abeec0ad 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -314,6 +314,7 @@ Each of these cost a real integrator or a real bug. - **The handler-tree caches engage only on the *second* evaluation of a given script on a given engine.** A host that builds a fresh engine per operation never reaches them, by design. This concerns that cross-run carry-over only — caches which are not engine-scoped still pay back, and CLR member resolution in particular now survives across engines through the shared `TypeResolver` described above, which is the main reason a fresh engine per operation costs less than it used to. `Engine.Advanced.RestoreGlobalSnapshot` deliberately **preserves** these caches (`_scriptStatementLists`, `_functionDefinitions`, `_evaluatedScripts`) while resetting the globals — reaching them from a host that used to need a fresh engine per evaluation is the entire point of that API, and `Jint.Tests/Runtime/GlobalSnapshotInternalsTests.cs` pins it. - **A warmed member-read site retains its last receiver.** The prototype-method inline cache stores the receiver, its direct prototype and the resolved descriptor on the handler node (`JintMemberExpression._cachedProtoReceiver`), and only replaces that entry when the same site later caches a *different* receiver — a miss that does not qualify for caching leaves the old one in place. Handler trees are engine-owned and survive between evaluations on a re-running engine, so a pooled engine can hold one host object alive per warmed call site until its next run. Nothing clears these: `Engine.Advanced.ResetCallStack` releases call frames only. A host whose receivers wrap large native state that must not outlive a run should drop the engine rather than pool it. **A warmed *call* site retains its last callee the same way, and one of the two caches is heavier than it looks.** `JintCallExpression._fastCallee` (the built-in fast-call lane) usually holds a realm intrinsic, which the realm roots anyway — but the field is written for whatever `Function` an eligible site dispatched, before and regardless of the shape verdict, so a site with at most two arguments and no spread calling an *interpreted* function pins that too. `_regCallee` (the register-argument lane for interpreted callees) holds a `ScriptFunction` by construction, and a `ScriptFunction` holds the environment it closed over — so a warmed site pins one closure instance, and transitively whatever that closure captured, for the engine's lifetime. It is the same bounded, one-entry-per-site hazard, with the same remedy: drop the engine rather than pool it when the retained graph matters. The bound is per site, not per callee: a polymorphic site overwrites its entry rather than accumulating. The sibling `_regProbedCallee` records what the probe last examined so a rejected callee is not re-probed on every dispatch, and it retains that reference too — for an accepted callee it is the same object as `_regCallee`, so the ceiling stays one live callee per site either way. **Which sites are in the retaining set is decided at build time**, by `_fastArgsEligible` and `_regLaneEligible`: a site with five arguments, or with a spread, can arm neither lane and so records nothing at all. Note also that a site only enters the set once its handler tree outlives the evaluation, i.e. from the second run of the same program on the same engine — a host re-parsing its source every time never warms one, and a `WeakReference` test written against `Execute(string)` therefore proves nothing. - **`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. +- **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. - **Saturated sentinels register nothing.** `MaxStatements(int.MaxValue)`, `LimitMemory(long.MaxValue)` and `TimeoutInterval(TimeSpan.MaxValue)` — and any non-positive value, including `MaxStatements()`'s own parameter default — produce exactly the same engine as never calling the method, and additionally *remove* any previously registered constraint of that kind. A host spelling "effectively unlimited" that way has no limit, not a very large one. - **`RestoreGlobalSnapshot` bumps version counters, it never restores them.** `_propertiesVersion`, `GlobalEnvironment._lexicalMutations`, `Engine._envBindingInjectionEpoch` and `EventLoop.Generation` are what every inline cache — and, for the last one, every queued job — validates against, so putting a counter *back* could make an entry built before the capture compare equal again and be revalidated against state it never saw. Anything added to the restore path obeys the same rule. The API's other half is its honesty: it reverts the global *binding table*, and explicitly not intrinsic/prototype mutations, object graphs behind restored bindings, host CLR state (including `Engine.Advanced.HostDefined`, which a pooled engine keeps across a restore and the host swaps per request itself), `Symbol.for`, or the module registry — it is a configuration-reuse primitive, not an isolation boundary, and the non-guarantees are pinned as surviving in `Jint.Tests.PublicInterface/GlobalSnapshotTests.cs`. Since bare identifiers resolve through the global's whole prototype chain, surviving intrinsic pollution is now a surviving **binding** as well as a surviving property: `Object.prototype.leaked = 1` in one cycle makes `leaked` resolvable as a free identifier in the next, where before it was readable only as `globalThis.leaked` and `typeof leaked` answered `"undefined"`. The global's own `[[Prototype]]` is a different matter and *is* captured and restored. - **Discarding the event loop is a fence, not a flush.** `EventLoop.Clear()` can only throw away what is already queued, and the case that matters is the one where nothing is: a fire-and-forget async function suspended on a CLR `Task` enqueues its settle whenever that task happens to complete, which can be after a restore — and the resumed body then writes the previous cycle's data into the restored globals, a cross-cycle channel the fresh-engine-per-evaluation pattern never had. The fix is a generation captured at promise **registration** (engine thread), carried in the `EventLoopJob`, and checked at **dequeue** (engine thread): both ends are ordered by the single-thread contract, where a check inside the settle closure would race the restore. Any new enqueue path must stamp the registering cycle's generation, not the current one, whenever the two can differ. The consequence for hosts is real and documented: a promise registered before a restore never settles into the engine afterwards. diff --git a/Jint.Tests.PublicInterface/BoundFunctionChainWalkTests.cs b/Jint.Tests.PublicInterface/BoundFunctionChainWalkTests.cs index befe165716..44e570d0c8 100644 --- a/Jint.Tests.PublicInterface/BoundFunctionChainWalkTests.cs +++ b/Jint.Tests.PublicInterface/BoundFunctionChainWalkTests.cs @@ -53,7 +53,7 @@ public class BoundFunctionChainWalkTests ["new"] = ("new f(); return 'constructed';", StackExhausted), ["Array.from with the chain as this"] = ("Array.from.call(f, []); return 'constructed';", StackExhausted), ["super() into the chain"] = ("class D extends Object { constructor() { super(); } } Object.setPrototypeOf(D, f); new D(); return 'constructed';", StackExhausted), - ["a ShadowRealm call of the chain"] = ("new ShadowRealm().evaluate('(h) => h()')(f); return 'called';", "TypeError:Cross-Realm Error: Cross-Realm Error: Maximum call stack size exceeded"), + ["a ShadowRealm call of the chain"] = ("new ShadowRealm().evaluate('(h) => h()')(f); return 'called';", "TypeError:Cross-Realm Error: Maximum call stack size exceeded"), }; [Fact] diff --git a/Jint.Tests.PublicInterface/HostNativeRecursionGuardTests.cs b/Jint.Tests.PublicInterface/HostNativeRecursionGuardTests.cs index 6813f6470b..948c4c7019 100644 --- a/Jint.Tests.PublicInterface/HostNativeRecursionGuardTests.cs +++ b/Jint.Tests.PublicInterface/HostNativeRecursionGuardTests.cs @@ -3,6 +3,7 @@ using Jint; using Jint.Native; using Jint.Native.Object; +using Jint.Runtime; using Jint.Runtime.Interop; namespace Jint.Tests.PublicInterface; @@ -206,6 +207,58 @@ public void NativeForwardingChainRaisesACatchableErrorAndTheEngineRecovers(strin }, maxStackSize: ForwardingStack); } + /// + /// Two recursions across a ShadowRealm boundary, each a native frame per level, where a failure is + /// copied into a TypeError at every level on its way back up + /// (https://tc39.es/proposal-shadowrealm/#sec-create-type-error-copy). The probe raised its + /// RangeError at the bottom of both, and the host process still ended: the copy was thrown from + /// inside the catch handling the failure it copies, so each level nested one more exception + /// dispatch on top of every frame down to the bottom, and nothing probes an exception dispatch. Five + /// hundred round trips on a 1 MB thread were enough on both .NET 10 and .NET Framework; the first row + /// makes ten times that. + /// + /// The first row is WrappedFunction's [[Call]] over a chain script builds by passing a + /// function back and forth; the second is WrappedFunctionCreate, which reads name to copy it + /// and here finds a getter that wraps its own receiver again, so it has no depth to choose at all. The + /// message is asserted whole because the copy's mark used to be added once per level, which made the + /// message — and every copy on the way up — longer with each one. + /// + /// + /// The engine opts into StackOverflowGuard, which is off by default on this branch: without it + /// nothing turns the recursion into a RangeError in the first place, and both rows end the process + /// whether or not the copy is thrown from inside its handler. + /// + /// + public static TheoryData CrossRealmCopyChains => new() + { + { + "wrapped call", + "const sr = new ShadowRealm(); const id = sr.evaluate('x => x'); let f = function () { return 1; }; for (let i = 0; i < 5000; i++) f = id(f); f();" + }, + { + "wrapped create", + "const sr = new ShadowRealm(); const g = function () {}; Object.defineProperty(g, 'name', { get: sr.evaluate('(function () {})') }); sr.evaluate('f => f')(g);" + }, + }; + + [Theory] + [MemberData(nameof(CrossRealmCopyChains))] + public void AFailureCopiedAcrossAShadowRealmBoundaryAtEveryLevelIsCatchable(string route, string script) + { + _ = route; + DedicatedThread.Run(() => + { + using var engine = Guarded(); + + var thrown = engine.Invoking(e => e.Execute(script)).Should().Throw().Which; + thrown.Message.Should().Be("Cross-Realm Error: Maximum call stack size exceeded"); + engine.SetValue("thrown", thrown.Error); + engine.Evaluate("thrown instanceof TypeError").AsBoolean().Should().BeTrue(); + + engine.Evaluate("6 * 7").AsNumber().Should().Be(42); + }, maxStackSize: SmallStack); + } + [Fact] public void HostCallableRecursionRaisesACatchableErrorAndTheEngineRecovers() { diff --git a/Jint.Tests/Runtime/ShadowRealmErrorCopyTests.cs b/Jint.Tests/Runtime/ShadowRealmErrorCopyTests.cs new file mode 100644 index 0000000000..233c97e6e6 --- /dev/null +++ b/Jint.Tests/Runtime/ShadowRealmErrorCopyTests.cs @@ -0,0 +1,160 @@ +#nullable enable + +namespace Jint.Tests.Runtime; + +/// +/// A failure that leaves a ShadowRealm is replaced by a fresh TypeError of the calling realm, +/// and https://tc39.es/proposal-shadowrealm/#sec-create-type-error-copy says building it "must not cause any +/// ECMAScript code execution". The copy's message is the host's to choose, so it may describe the value that +/// was thrown — but only from what can be read without calling anything: the string a primitive is, and an +/// error's name and message where each is a data property holding a string. +/// +/// ShadowRealm.prototype.evaluate used to format the thrown value with ToString, which calls +/// toString, @@toPrimitive, a name or message getter and every proxy trap on the +/// way — and when one of those threw, what reached the caller was that exception, an object of the shadow +/// realm, instead of the TypeError. +/// +/// +public class ShadowRealmErrorCopyTests +{ + /// + /// copyOf answers "<kind>|<message>|ran=<n>", where ran counts every call a + /// row's script makes into its own code, so a failure says both what crossed and whether anything ran. + /// + private const string Preamble = """ + var sr = new ShadowRealm(); + sr.evaluate('var ran = 0;'); + + function copyOf(source) { + try { + sr.evaluate(source); + return 'no throw'; + } catch (e) { + var kind = e instanceof TypeError ? 'TypeError' : 'not a TypeError of this realm'; + return kind + '|' + e.message + '|ran=' + sr.evaluate('ran'); + } + } + """; + + private const string NotAnError = "Cross-Realm Error: an object that is not an Error was thrown"; + + /// A handler every one of whose traps counts itself before forwarding to Reflect. + private const string CountingHandler = """ + var handler = {}; + ['get', 'set', 'has', 'deleteProperty', 'defineProperty', 'getOwnPropertyDescriptor', 'ownKeys', + 'getPrototypeOf', 'setPrototypeOf', 'isExtensible', 'preventExtensions', 'apply', 'construct'] + .forEach(function (trap) { + handler[trap] = function () { ran++; return Reflect[trap].apply(null, arguments); }; + }); + """; + + private static string CopyOf(string source) + { + using var engine = new Engine(); + engine.Execute(Preamble); + engine.SetValue("source", source); + return engine.Evaluate("copyOf(source)").AsString(); + } + + [Theory] + [InlineData("throw { toString() { ran++; return 'custom'; } };")] + [InlineData("throw { toString() { ran++; throw new RangeError('from toString'); } };")] + [InlineData("throw { [Symbol.toPrimitive]() { ran++; return 'primitive'; } };")] + [InlineData("throw { valueOf() { ran++; return 1; }, toString: undefined };")] + [InlineData("throw { message: 'looks like an error' };")] + public void AThrownObjectIsNotConvertedToAString(string source) + { + CopyOf(source).Should().Be("TypeError|" + NotAnError + "|ran=0"); + } + + [Theory] + [InlineData( // a message getter + "var e = new Error('x'); Object.defineProperty(e, 'message', { get() { ran++; return 'from getter'; } }); throw e;", + "Error")] + [InlineData( // a throwing message getter + "var e = new Error('x'); Object.defineProperty(e, 'message', { get() { ran++; throw new RangeError('from getter'); } }); throw e;", + "Error")] + [InlineData( // a name getter + "var e = new Error('x'); Object.defineProperty(e, 'name', { get() { ran++; return 'FromGetter'; } }); throw e;", + "Error: x")] + [InlineData( // a message getter on a subclass prototype + "class E extends Error { get message() { ran++; return 'from getter'; } } throw new E();", + "Error")] + [InlineData( // a message that is not a string + "var e = new Error(); e.message = { toString() { ran++; return 'from toString'; } }; throw e;", + "Error")] + [InlineData( // a proxy on the error's prototype chain + CountingHandler + "var e = new Error('x'); Object.setPrototypeOf(e, new Proxy(Error.prototype, handler)); ran = 0; throw e;", + "Error: x")] + public void AnErrorsNameAndMessageAreReadWithoutCallingAnything(string source, string described) + { + CopyOf(source).Should().Be("TypeError|Cross-Realm Error: " + described + "|ran=0"); + } + + [Theory] + [InlineData("throw new Proxy({}, handler);")] + [InlineData("throw new Proxy(new Error('proxied'), handler);")] + [InlineData("throw new Proxy(function () {}, handler);")] + public void AThrownProxyRunsNoTrap(string source) + { + CopyOf(CountingHandler + source).Should().Be("TypeError|" + NotAnError + "|ran=0"); + } + + /// + /// What the copy says for an ordinary error and for a primitive is unchanged, and is pinned so that making + /// the copy safe does not quietly cost the diagnostic. + /// + [Theory] + [InlineData("throw new ReferenceError('aaa');", "ReferenceError: aaa")] + [InlineData("throw new Error();", "Error")] + [InlineData("var e = new Error('x'); e.name = ''; throw e;", "x")] + [InlineData("throw 42;", "42")] + [InlineData("throw 'boom';", "boom")] + [InlineData("throw Symbol('s');", "Symbol(s)")] + [InlineData("throw undefined;", "undefined")] + [InlineData("throw 10n;", "10")] + public void AnOrdinaryErrorOrAPrimitiveIsStillDescribed(string source, string described) + { + CopyOf(source).Should().Be("TypeError|Cross-Realm Error: " + described + "|ran=0"); + } + + /// + /// A failure copied out of a shadow realm nested inside another one is copied again on the way out of the + /// outer realm. The copy it crosses with already says it crossed, so it is marked once, exactly as a chain + /// of wrapped functions is. + /// + [Theory] + [InlineData("new ShadowRealm().evaluate(\"throw new Error('boom')\");")] + [InlineData("new ShadowRealm().evaluate(\"new ShadowRealm().evaluate('throw new Error(\\\\'boom\\\\')')\");")] + public void ACopyCrossingNestedRealmsIsMarkedOnce(string source) + { + CopyOf(source).Should().Be("TypeError|Cross-Realm Error: Error: boom|ran=0"); + } + + /// + /// importValue copies a failed import through + /// https://tc39.es/proposal-shadowrealm/#sec-import-value-error-functions, the same operation, so its + /// rejection describes what the module threw in the same terms and calls nothing to do it. + /// + [Theory] + [InlineData("throw new RangeError('module failed');", "Cross-Realm Error: RangeError: module failed")] + [InlineData("throw { toString() { globalThis.ran = (globalThis.ran || 0) + 1; return 'custom'; } };", NotAnError)] + public void AFailedImportIsCopiedWithoutCallingAnything(string moduleSource, string expectedMessage) + { + using var engine = new Engine(); + engine.Modules.Add("throws", moduleSource); + + engine.Execute(""" + var sr = new ShadowRealm(); + var outcome = 'pending'; + sr.importValue('throws', 'x').then( + function () { outcome = 'fulfilled'; }, + function (e) { + var kind = e instanceof TypeError ? 'TypeError' : 'not a TypeError of this realm'; + outcome = kind + '|' + e.message + '|ran=' + sr.evaluate('globalThis.ran || 0'); + }); + """); + + engine.Evaluate("outcome").AsString().Should().Be("TypeError|" + expectedMessage + "|ran=0"); + } +} diff --git a/Jint/Native/ShadowRealm/ShadowRealm.cs b/Jint/Native/ShadowRealm/ShadowRealm.cs index ce47f3eb8a..946eee1a26 100644 --- a/Jint/Native/ShadowRealm/ShadowRealm.cs +++ b/Jint/Native/ShadowRealm/ShadowRealm.cs @@ -211,7 +211,7 @@ internal JsValue PerformShadowRealmEvalInternal(in Prepared