diff --git a/Jint.Tests/Runtime/SlotLocationCacheTests.cs b/Jint.Tests/Runtime/SlotLocationCacheTests.cs index 23e6972a7e..ca7081f8ed 100644 --- a/Jint.Tests/Runtime/SlotLocationCacheTests.cs +++ b/Jint.Tests/Runtime/SlotLocationCacheTests.cs @@ -55,6 +55,187 @@ function inner() { Assert.Equal(4 * (100 + 100_000), result); } + [Fact] + public void AlternatingClosureInstancesReadingOuterBindingsStayCorrect() + { + var engine = new Engine(); + // hops-1 closure reads through the chain memo: two closure instances of the same + // definition alternate at ONE callsite, so the shared node's pinned chain flips + // between the instances' environments on every call and must re-validate, not + // serve the other instance's binding + var result = engine.Evaluate(""" + (function () { + function make(table) { + return function () { + var acc = ''; + for (var i = 0; i < 8; i++) { acc += table[i % table.length]; } + return acc; + }; + } + var a = make(['A']), b = make(['B']); + var got = ''; + for (var r = 0; r < 3; r++) { got += a() + '|' + b() + ';'; } + return got; + })() + """).AsString(); + + Assert.Equal("AAAAAAAA|BBBBBBBB;AAAAAAAA|BBBBBBBB;AAAAAAAA|BBBBBBBB;", result); + } + + [Fact] + public void SloppyDirectEvalShadowingVarInvalidatesChainMemoMidLoop() + { + var engine = new Engine(); + // sloppy direct eval injects `captured` into the enclosing FUNCTION env mid-loop: + // the pre-eval iterations pin a chain memo to the outer binding; post-injection the + // epoch bump must force the walk to see the new shadowing binding immediately + var result = engine.Evaluate(""" + (function () { + var captured = 'outer'; + function reader() { + var seen = ''; + for (var i = 0; i < 6; i++) { + seen += captured; + if (i === 2) { eval("var captured = 'inner';"); } + seen += ','; + } + return seen; + } + return reader() + '|' + captured; + })() + """).AsString(); + + // i=0..2 read the outer binding (the eval at i===2 runs AFTER the read); the + // injected var initializes to 'inner' and shadows from the i=3 read onward, and + // the outer binding must remain untouched + Assert.Equal("outer,outer,outer,inner,inner,inner,|outer", result); + } + + [Fact] + public void SloppyDirectEvalHoistedVarShadowsFromInjectionOn() + { + var engine = new Engine(); + // hoisting-only variant: `var captured;` without initializer still creates the + // shadowing binding (initialized to undefined by EvalDeclarationInstantiation), + // flipping subsequent memo-guarded reads from 'outer' to undefined + var result = engine.Evaluate(""" + (function () { + var captured = 'outer'; + function reader() { + var seen = ''; + for (var i = 0; i < 4; i++) { + seen += captured + ','; + if (i === 1) { eval("var captured;"); } + } + return seen; + } + return reader(); + })() + """).AsString(); + + Assert.Equal("outer,outer,undefined,undefined,", result); + } + + [Fact] + public void WithBlockAroundClosureReadIsNeverSkipped() + { + var engine = new Engine(); + // the same identifier node runs both under a with-ObjectEnvironment and without it: + // a chain pinned outside the with must never validate inside it (the with-object can + // shadow dynamically), and property addition mid-loop must be honored immediately + var result = engine.Evaluate(""" + (function () { + var captured = 'free'; + var obj = {}; + function reader(useWith) { + var seen = ''; + if (useWith) { + with (obj) { + for (var i = 0; i < 4; i++) { + seen += captured + ','; + if (i === 1) { obj.captured = 'shadowed'; } + } + } + } else { + for (var j = 0; j < 2; j++) { seen += captured + ','; } + } + return seen; + } + // warm the memo without the with-block, then run under it, then without again + return reader(false) + '|' + reader(true) + '|' + reader(false); + })() + """).AsString(); + + Assert.Equal("free,free,|free,free,shadowed,shadowed,|free,free,", result); + } + + [Fact] + public void DeeplyNestedClosureChainsResolveCorrectly() + { + var engine = new Engine(); + // reads at hop distances beyond MaxChainDepth must keep falling through to full + // resolution and stay correct; distances within the bound exercise the memo + var result = engine.Evaluate(""" + (function () { + var deep = 'root'; + function l1() { + var a1 = 1; + function l2() { + var a2 = 2; + function l3() { + var a3 = 3; + function l4() { + var a4 = 4; + function l5() { + var acc = ''; + for (var i = 0; i < 3; i++) { + acc += deep + (a1 + a2 + a3 + a4); + } + return acc; + } + return l5(); + } + return l4(); + } + return l3(); + } + return l2(); + } + return l1() + '|' + l1(); + })() + """).AsString(); + + Assert.Equal("root10root10root10|root10root10root10", result); + } + + [Fact] + public void PooledLoopEnvironmentsReattachedAcrossEntriesKeepMemoValid() + { + var engine = new Engine(); + // let-bound loop bodies get per-node pooled block environments re-attached across + // loop entries: the pinned chain's start link is the SAME instance on a fresh entry, + // so the memo re-validates against current outer pointers and must stay correct + // across function instance switches too + var result = engine.Evaluate(""" + (function () { + function make(base) { + return function () { + var acc = 0; + for (let i = 0; i < 50; i++) { + const step = 1; + acc += base + step; + } + return acc; + }; + } + var a = make(10), b = make(1000); + return a() + b() + a(); + })() + """).AsNumber(); + + Assert.Equal(50 * 11 + 50 * 1001 + 50 * 11, result); + } + #if NET [Fact] public void SlotLanesSurviveRepeatedPreparedEvaluations() diff --git a/Jint/Runtime/Environments/SlotLocationCache.cs b/Jint/Runtime/Environments/SlotLocationCache.cs index cc5e8eb8b0..29ef6152b1 100644 --- a/Jint/Runtime/Environments/SlotLocationCache.cs +++ b/Jint/Runtime/Environments/SlotLocationCache.cs @@ -2,6 +2,50 @@ namespace Jint.Runtime.Environments; +/// +/// Immutable snapshot of a successful hops 1-3 slot-cache reachability walk: from +/// , following exactly the pinned links, the cached slot environment was +/// reached and none of the probed intermediates (, , +/// ) owned the name. While every link still matches by identity and +/// is unchanged, the per-hop shadow probes are +/// provably still false and the walk can be skipped entirely. +/// +/// +/// Validity reasoning, mirroring JintIdentifierExpression.NestedChainMemo (the global +/// read cache's chain memo) with a declarative terminal instead of the global environment: +/// a declarative environment's name set is a deterministic function of its defining AST +/// node/definition — every reuse channel (per-function-instance env reuse, the recursive env +/// pool, definition-level dynamic-function envs, per-node block/loop/catch env caches, the +/// per-source eval env pool) re-initializes an instance with the identical name set. The only +/// mutations that grow a PRE-EXISTING environment's name set (sloppy direct eval var/function +/// hoisting, AnnexB block-function var-scope copies) bump the injection epoch; deletions only +/// shrink it (a pinned "does not own the name" stays true), and an environment's class +/// (ObjectEnvironment vs declarative) is immutable. Chain-LINK identity is required, not just +/// the start: pooled environments are re-attached under different outers across entries, and +/// validation follows the CURRENT _outerEnv pointers, so a re-attached or replaced link +/// falls through to the walk. A memo is published as one immutable object through a single +/// reference field so cross-engine shared handler trees observe consistent snapshots; a memo +/// pinned by another engine always fails the start-identity check because environments are +/// per-engine. The terminal is intentionally NOT pinned: the memo's claim is only about the +/// probed intermediates, so it stays valid when the node re-resolves to a new slot env behind +/// the same intermediates. +/// +internal sealed class SlotChainMemo +{ + internal readonly Environment Start; + internal readonly Environment? Next1; + internal readonly Environment? Next2; + internal readonly int InjectionEpoch; + + internal SlotChainMemo(Environment start, Environment? next1, Environment? next2, int injectionEpoch) + { + Start = start; + Next1 = next1; + Next2 = next2; + InjectionEpoch = injectionEpoch; + } +} + /// /// Per-AST-node cache of a binding's slot location (environment + slot index), shared by the /// identifier read fast path and the numeric read-modify-write discard fast paths. @@ -23,7 +67,10 @@ namespace Jint.Runtime.Environments; /// eval can inject a shadowing var into an enclosing function environment — so the hit /// walk re-probes every intermediate declarative environment (pure) and refuses to skip /// over an (a with-object can gain a shadowing property -/// at any time, and probing it would be observable through proxy traps). +/// at any time, and probing it would be observable through proxy traps). A successful walk +/// is memoized as a so steady-state closure reads validate with +/// a handful of reference compares instead of re-walking; the walk stays authoritative for +/// every miss. /// Nodes whose binding can never be slot-stored (global/object/dictionary environments) /// disable themselves permanently so failed attempts don't re-walk the chain. /// @@ -35,6 +82,7 @@ internal struct SlotLocationCache private DeclarativeEnvironment? _cachedSlotEnv; private int _cachedSlotIndex; private bool _disabled; + private SlotChainMemo? _chainMemo; /// /// Resolves the slot location for against the current lexical @@ -77,9 +125,10 @@ public bool TryResolve( /// /// Out-of-line tail for everything that is not a hop-0 hit or a permanent decline: the - /// bounded hops 1-3 reachability walk, the decline check for nodes with a stale cached - /// env, and first-time population. Kept out of so the - /// aggressively-inlined fast path stays small in every consuming lane. + /// bounded hops 1-3 reachability check (chain memo, then walk), the decline check for + /// nodes with a stale cached env, and first-time population. Kept out of + /// so the aggressively-inlined fast path stays small in every + /// consuming lane. /// [MethodImpl(MethodImplOptions.NoInlining)] private bool TryResolveNonLocal( @@ -92,7 +141,7 @@ private bool TryResolveNonLocal( var cached = _cachedSlotEnv; if (cached is not null && ReferenceEquals(cached._engine, engine) - && CanReachAtOuterHop(env, cached, name.Key)) + && CanReachAtOuterHop(engine, env, cached, name.Key, ref _chainMemo)) { slotEnv = cached; slotIndex = _cachedSlotIndex; @@ -114,19 +163,57 @@ private bool TryResolveNonLocal( return false; } - return ResolveAndPopulate(env, name, out slotEnv, out slotIndex); + return ResolveAndPopulate(engine, env, name, out slotEnv, out slotIndex); } /// - /// Bounded hops 1-3 reachability walk for a slot-cache probe whose caller has already + /// Bounded hops 1-3 reachability check for a slot-cache probe whose caller has already /// rejected hop 0 ( itself is not ). /// Because hop 0 failed, the current environment sits BETWEEN the reader and the cached - /// env and must be probed like any other intermediate before hopping outward. + /// env and must be probed like any other intermediate before hopping outward. A valid + /// answers with reference compares alone (see its remarks for + /// why identity + epoch freeze the probe results); everything else takes the walk, which + /// re-publishes the memo on success. /// [MethodImpl(MethodImplOptions.NoInlining)] - internal static bool CanReachAtOuterHop(Environment env, DeclarativeEnvironment cached, Key key) + internal static bool CanReachAtOuterHop(Engine engine, Environment env, DeclarativeEnvironment cached, Key key, ref SlotChainMemo? memoSlot) + { + var memo = memoSlot; + if (memo is not null + && ReferenceEquals(env, memo.Start) + && engine._envBindingInjectionEpoch == memo.InjectionEpoch) + { + // Re-derive the chain from the CURRENT _outerEnv pointers against the pinned + // links: identity per link leaves no room for an inserted environment, and a + // pooled link re-attached under a different outer fails here and re-walks. + var next = env._outerEnv; + if (memo.Next1 is null + ? ReferenceEquals(next, cached) + : ReferenceEquals(next, memo.Next1) + && (memo.Next2 is null + ? ReferenceEquals(memo.Next1._outerEnv, cached) + : ReferenceEquals(memo.Next1._outerEnv, memo.Next2) + && ReferenceEquals(memo.Next2._outerEnv, cached))) + { + return true; + } + } + + return WalkAndMemoize(engine, env, cached, key, ref memoSlot); + } + + /// + /// The authoritative bounded walk: probes every intermediate declarative environment + /// (pure ) and refuses to cross an + /// . On success the probed intermediates are published as + /// a so subsequent reads on the same chain skip the probes. + /// + [MethodImpl(MethodImplOptions.NoInlining)] + private static bool WalkAndMemoize(Engine engine, Environment env, DeclarativeEnvironment cached, Key key, ref SlotChainMemo? memoSlot) { var search = env; + Environment? next1 = null; + Environment? next2 = null; for (var hops = 1; hops < MaxChainDepth; hops++) { if (search is ObjectEnvironment) @@ -155,8 +242,18 @@ internal static bool CanReachAtOuterHop(Environment env, DeclarativeEnvironment if (ReferenceEquals(search, cached)) { + memoSlot = new SlotChainMemo(env, next1, next2, engine._envBindingInjectionEpoch); return true; } + + if (hops == 1) + { + next1 = search; + } + else + { + next2 = search; + } } return false; @@ -164,6 +261,7 @@ internal static bool CanReachAtOuterHop(Environment env, DeclarativeEnvironment [MethodImpl(MethodImplOptions.NoInlining)] private bool ResolveAndPopulate( + Engine engine, Environment env, Environment.BindingName name, out DeclarativeEnvironment slotEnv, @@ -178,6 +276,9 @@ private bool ResolveAndPopulate( // bindings are never slot-stored — reaching a non-declarative environment before the // binding means this node can never be safely slot-cached, without touching it. var record = env; + Environment? next1 = null; + Environment? next2 = null; + var depth = 0; while (record is not null) { if (record is not DeclarativeEnvironment declarativeEnvironment) @@ -192,6 +293,13 @@ private bool ResolveAndPopulate( { _cachedSlotEnv = declarativeEnvironment; _cachedSlotIndex = index; + if ((uint) (depth - 1) < MaxChainDepth - 1) + { + // Found at hops 1-3: every traversed environment was declarative and + // did not own the name — exactly the walk's success condition, so the + // memo can be published without a second walk on the next read. + _chainMemo = new SlotChainMemo(env, next1, next2, engine._envBindingInjectionEpoch); + } slotEnv = declarativeEnvironment; slotIndex = index; return true; @@ -201,7 +309,17 @@ private bool ResolveAndPopulate( break; } + if (depth == 1) + { + next1 = record; + } + else if (depth == 2) + { + next2 = record; + } + record = record._outerEnv; + depth++; } // the binding for this node can never be slot-stored; stop attempting diff --git a/Jint/Runtime/Interpreter/Expressions/JintIdentifierExpression.cs b/Jint/Runtime/Interpreter/Expressions/JintIdentifierExpression.cs index c4f0d14194..f60136d5b2 100644 --- a/Jint/Runtime/Interpreter/Expressions/JintIdentifierExpression.cs +++ b/Jint/Runtime/Interpreter/Expressions/JintIdentifierExpression.cs @@ -23,6 +23,12 @@ internal sealed class JintIdentifierExpression : JintExpression private DeclarativeEnvironment? _cachedSlotEnv; private int _cachedSlotIndex = -1; + // Chain memo for the hops 1-3 slot reads (closure reads of outer bindings): pins the exact + // chain links a successful reachability walk probed, so steady-state loop reads validate + // with reference compares + the injection epoch instead of re-probing every intermediate. + // See SlotChainMemo for the soundness argument; the walk stays authoritative for misses. + private SlotChainMemo? _slotChainMemo; + // Version-based inline cache for global bindings: when resolution (from any scope depth) // lands on a plain writable data property of the real GlobalObject, remember the // descriptor. Valid while the global object's own-property shape and the set of global @@ -158,9 +164,11 @@ public override JsValue GetValue(EvaluationContext context) // cached resolving env IS the current lexical environment, so it gets a straight-line // identity check before any loop machinery and needs no shadow probes — no environment // sits between the reader and the binding. Reads at hops 1-3 (closure reads of outer - // bindings) take the out-of-line bounded walk, which re-probes every intermediate - // declarative environment and refuses to cross an ObjectEnvironment; see - // SlotLocationCache for why those probes are load-bearing. + // bindings) take the out-of-line bounded reachability check: a pinned chain memo + // answers with reference compares, everything else re-walks with per-intermediate + // shadow probes and refuses to cross an ObjectEnvironment; see SlotLocationCache and + // SlotChainMemo for why those probes (and the memo's identity + epoch guard) are + // load-bearing. // Engine-identity gate: a Prepared