From 366ef6d5bc78e4bce717e607b98c48b6f7c97429 Mon Sep 17 00:00:00 2001 From: Marko Lahma Date: Thu, 17 Sep 2026 19:17:59 +0300 Subject: [PATCH] Async and generators: a suspended frame dereferences nothing the suspension produced (backport of #4088) Backport of PR #4088 (commit bda631c876a9ad96458017b7cbc34e9b2813433f) from main. Closes the 4.x half of #4086. `await` and `yield` suspend by returning a plain `JsValue.Undefined`, and the enclosing member link turns that into a sentinel `Reference(undefined, undefined)` that every consumer must recognise before reading. Nine did not, so they read `undefined.undefined` and raised a `TypeError` inside a frame that was already suspended. `AsyncBlockStart` swallows that throw -- the state is still `SuspendedAwait` -- but not before `JintStatementList.LeavingOnException` had cleared the statement-list resume position, so the resume replayed the body from statement 0: one extra run of every un-awaited side effect per suspension point, and a re-entrancy guard (`if (inited) return;`) silently truncating the rest. A generator has nothing to swallow it, so the same shapes threw straight out of `next()`. Two of the shapes are wrong answers rather than duplicated side effects: `(await p).x = 1` rejected the promise, and `o[await k] = 1` assigned to the literal key `"undefined"` instead of the real one. One never terminated at all: `for await ((await p).a of it)`. `?.` is not the trigger, despite where the report put it. The guarded fast lane needs a literal property name, so every *computed* member read of an awaited or yielded value falls through to the unguarded one -- `(await p)[0]` and `(await p)[k]` alike. Adds the missing `context.IsSuspended()` checks to `JintMemberExpression.GetValue`'s fall-through lane, `typeof`, `delete`, `++`/`--`, both assignment forms, the two object-pattern branches of `ProcessPatterns` and the non-destructuring for-in/for-of head, and stops the exception filter clearing the resume position while the frame is suspended. The per-site checks are the fix; the filter guard is only the net, because a statement that throws never reaches the line that records its position. Adapted for 4.x: - The interpreter `AGENTS.md` gotcha and the `migrating-to-v5.md` section are dropped: neither file exists on this branch, and 4.x's root `AGENTS.md` is the unsplit monolith. - Tests transcribed from NUnit to xUnit, which is what 4.x's `Jint.Tests` still is; the attribute is the only difference, the assertions are AwesomeAssertions on both branches. - Nothing else. All nine production sites are at the same shape on this branch -- `JintMemberExpression` has main's identical three-lane structure, only offset -- so every hunk applied without conflict. Evidence on this branch, `Jint.Tests/Runtime/SuspendedOptionalChainTests.cs` run against the unfixed 4.x tree: **28 failed, 6 passed of 34 on net10.0 and again on net472**, with the 35th, `AForAwaitOfHeadTargetingAMemberOfAnAwaitedValueRunsItsSideEffectsOnce`, hanging the runner outright (it is excluded from those counts because an unterminating test cannot be counted; the test host had to be killed). That is the same 28-plus-one-hang split main measured. After the fix: **35/35 on net10.0 and 35/35 on net472**, the hang included. The six that pass unfixed are the `...IsAControl` cases, which are there to bound the claim rather than to evidence it. Co-Authored-By: Claude Opus 5 (1M context) --- .../Runtime/SuspendedOptionalChainTests.cs | 405 ++++++++++++++++++ ...estructuringPatternAssignmentExpression.cs | 19 + .../Expressions/JintAssignmentExpression.cs | 32 +- .../Expressions/JintMemberExpression.cs | 32 +- .../Expressions/JintUnaryExpression.cs | 24 +- .../Expressions/JintUpdateExpression.cs | 13 +- Jint/Runtime/Interpreter/JintStatementList.cs | 22 +- .../Statements/JintForInForOfStatement.cs | 19 + 8 files changed, 555 insertions(+), 11 deletions(-) create mode 100644 Jint.Tests/Runtime/SuspendedOptionalChainTests.cs diff --git a/Jint.Tests/Runtime/SuspendedOptionalChainTests.cs b/Jint.Tests/Runtime/SuspendedOptionalChainTests.cs new file mode 100644 index 0000000000..9eaaba46f4 --- /dev/null +++ b/Jint.Tests/Runtime/SuspendedOptionalChainTests.cs @@ -0,0 +1,405 @@ +#nullable enable + +namespace Jint.Tests.Runtime; + +/// +/// A frame that has suspended on an await or a yield must not dereference anything the +/// suspension produced, and the resume position it saved must survive an exception in flight through it. +/// +/// await and yield suspend by returning a plain , +/// and the enclosing member link turns that into a sentinel Reference(undefined, undefined). Every +/// consumer of that sentinel is supposed to bail on EvaluationContext.IsSuspended() before reading +/// it; the ones that did not read undefined.undefined, which raises a TypeError *while the +/// frame is suspended*. That throw is swallowed by AsyncBlockStart — the state is still +/// SuspendedAwait — but on its way out it wiped the statement-list resume position, so the resume +/// replayed the body from statement 0: one extra run of every un-awaited side effect per suspension point, +/// and a re-entrancy guard (if (inited) return;) silently truncating everything after it +/// (sebastienros/jint#4086). In a generator nothing swallows it and the TypeError comes straight +/// out of next(). +/// +/// +/// Each probe here pushes "pre" before the suspension and "post" after it and asserts the +/// log is exactly pre,post: a replayed body shows up as a second pre, one per suspension +/// point, which is the only symptom the swallowed throw leaves behind. The tests named +/// …Control pass on the unfixed tree and are here to bound the claim, not to evidence it. +/// +/// +public class SuspendedOptionalChainTests +{ + /// + /// Runs as the body of an async function bracketed by the pre/post + /// markers and returns "<report>|<log>". + /// + private static string AsyncProbe(string setup, string body, string report) => new Engine().Evaluate($$""" + var log = []; + {{setup}} + async function m() { + log.push('pre'); + {{body}} + log.push('post'); + return {{report}}; + } + m().then(function (r) { return String(r) + '|' + log.join(','); }); + """).UnwrapIfPromise().AsString(); + + /// + /// The synchronous-generator twin of : the suspension is a yield and + /// is what next() resumes it with, standing in for the awaited value. + /// + private static string GeneratorProbe(string setup, string body, string report, string sent) => new Engine().Evaluate($$""" + var log = []; + {{setup}} + function* g() { + log.push('pre'); + {{body}} + log.push('post'); + return {{report}}; + } + var it = g(); + it.next(); + var res = it.next({{sent}}); + String(res.value) + '|' + log.join(','); + """).AsString(); + + // ------------------------------------------------------------------ the reported repro + + private const string ReportedChain = """ + const h = { M: async function () { return { P: async function () { return { DATA: 0.05 }; } }; } }; + """; + + /// + /// The issue's own script. The guard makes the replay fatal rather than merely duplicated: the second + /// pass takes the early return, so the assignment never happens and everything after it is skipped. + /// + [Fact] + public void TheReportedChainCompletesOnceBehindAReEntrancyGuard() + { + new Engine().Evaluate($$""" + var log = []; + {{ReportedChain}} + const s = { f: false }; + async function m() { + if (s.f) { log.push("guard"); return; } + s.f = true; + log.push("pre"); + s.r = (await (await h?.M())?.P())?.DATA; + log.push("post"); + } + m().then(function () { return 'r=' + String(s.r) + ' log=' + log.join(','); }); + """).UnwrapIfPromise().AsString().Should().Be("r=0.05 log=pre,post"); + } + + /// + /// Without the guard the body still produces the right answer — the completed-await memo makes the + /// replayed awaits return instantly — so the whole defect is visible only in the un-awaited + /// side effects: one extra pre per suspension point, three of them in this chain. + /// + [Fact] + public void TheReportedChainRunsItsUnawaitedSideEffectsOnce() + { + AsyncProbe(ReportedChain, "var r = (await (await h?.M())?.P())?.DATA;", "r") + .Should().Be("0.05|pre,post"); + } + + // ------------------------------------------------------------------ the shapes that reach the unguarded lane + + /// + /// No optional chain anywhere: the guarded fast lane in JintMemberExpression.GetValue also + /// requires a literal string property, so any computed read of an awaited value falls to the same + /// unguarded lane. + /// + [Fact] + public void AComputedIndexReadOfAnAwaitedValueRunsItsSideEffectsOnce() + { + AsyncProbe("var p = Promise.resolve(['zero', 'one']);", "var r = (await p)[0];", "r") + .Should().Be("zero|pre,post"); + } + + /// + /// The key being a string changes nothing — it is the key being an expression rather than a + /// literal that disarms the fast lane's guard. + /// + [Fact] + public void AComputedNameReadOfAnAwaitedValueRunsItsSideEffectsOnce() + { + AsyncProbe("var p = Promise.resolve({ x: 7 }); var k = 'x';", "var r = (await p)[k];", "r") + .Should().Be("7|pre,post"); + } + + [Fact] + public void AnOptionalReadOfAnAwaitedValueRunsItsSideEffectsOnce() + { + AsyncProbe("var p = Promise.resolve({ x: 7 });", "var r = (await p)?.x;", "r") + .Should().Be("7|pre,post"); + } + + /// + /// The `?.(` link of the reported chain on its own: the call expression's callee lane already carries + /// its suspension check, so this half was never broken. Control. + /// + [Fact] + public void AnOptionalCallOnAnAwaitedReceiverIsAControl() + { + AsyncProbe(ReportedChain, "var r = (await h?.M())?.P();", "typeof r") + .Should().Be("object|pre,post"); + } + + /// + /// The suspension is in the argument rather than in the callee, so the call expression's own + /// suspension checks already cover it. Control. + /// + [Fact] + public void AnOptionalCallWithAnAwaitedArgumentIsAControl() + { + AsyncProbe("var h = { M: function (v) { return v + 1; } }; var p = Promise.resolve(1);", "var r = h?.M?.(await p);", "r") + .Should().Be("2|pre,post"); + } + + /// + /// Plain . member access on an awaited value takes the guarded fast lane and has always been + /// correct. Control: the fix must not be credited with it. + /// + [Fact] + public void APlainDotChainIsAControl() + { + AsyncProbe(ReportedChain, "var r = (await (await h.M()).P()).DATA;", "r") + .Should().Be("0.05|pre,post"); + } + + // ------------------------------------------------------------------ nothing may be read with the sentinel + + /// + /// The complementary half: when the property side suspends, the link hands on a + /// Reference(base, undefined) and the unguarded lane completed the read — an observable + /// base[undefined] probe, which a Proxy or a getter sees. + /// + [Fact] + public void AnAwaitedKeyNeverProbesTheBaseWithTheSuspensionSentinel() + { + AsyncProbe( + "var seen = []; var t = new Proxy({ x: 42 }, { get: function (o, k) { seen.push(String(k)); return o[k]; } }); var kp = Promise.resolve('x');", + "var r = t[await kp];", + "r + '/' + seen.join(',')").Should().Be("42/x|pre,post"); + } + + [Fact] + public void AnAwaitedKeyOnAnOptionalReadNeverProbesTheBaseWithTheSuspensionSentinel() + { + AsyncProbe( + "var seen = []; var t = new Proxy({ x: 42 }, { get: function (o, k) { seen.push(String(k)); return o[k]; } }); var kp = Promise.resolve('x');", + "var r = t?.[await kp];", + "r + '/' + seen.join(',')").Should().Be("42/x|pre,post"); + } + + /// + /// The same sentinel reached a simple assignment, which parks the resolved left-hand + /// Reference so a side-effecting key is not re-run. Parking an unfinished one made the resume + /// assign to the key "undefined" and never to the real one — a wrong answer rather than a + /// duplicated side effect. + /// + [Fact] + public void AnAwaitedKeyOnAnAssignmentTargetLandsOnTheRealKey() + { + AsyncProbe("var o = {}; var kp = Promise.resolve('x');", "o[await kp] = 1;", "JSON.stringify(o)") + .Should().Be("{\"x\":1}|pre,post"); + } + + // ------------------------------------------------------------------ the other consumers of the sentinel + + [Fact] + public void AnUpdateOfAMemberOfAnAwaitedValueRunsItsSideEffectsOnce() + { + AsyncProbe("var o = { x: 1 }; var p = Promise.resolve(o);", "(await p).x++;", "o.x") + .Should().Be("2|pre,post"); + } + + [Fact] + public void TypeOfAMemberOfAnAwaitedValueRunsItsSideEffectsOnce() + { + AsyncProbe("var p = Promise.resolve({ x: 1 });", "var t = typeof (await p).x;", "t") + .Should().Be("number|pre,post"); + } + + [Fact] + public void DeletingAMemberOfAnAwaitedValueRunsItsSideEffectsOnce() + { + AsyncProbe("var o = { x: 1 }; var p = Promise.resolve(o);", "delete (await p).x;", "'x' in o") + .Should().Be("false|pre,post"); + } + + [Fact] + public void ACompoundAssignmentToAMemberOfAnAwaitedValueRunsItsSideEffectsOnce() + { + AsyncProbe("var o = { x: 1 }; var p = Promise.resolve(o);", "(await p).x += 1;", "o.x") + .Should().Be("2|pre,post"); + } + + /// + /// This one did not merely replay: the sentinel Reference was parked as the assignment's + /// left-hand side and the resume tried to write through it, so the host saw the swallowed + /// TypeError after all — as a rejection of the async function's promise. + /// + [Fact] + public void AnAssignmentToAMemberOfAnAwaitedValueRunsItsSideEffectsOnce() + { + AsyncProbe("var o = { x: 1 }; var p = Promise.resolve(o);", "(await p).x = 9;", "o.x") + .Should().Be("9|pre,post"); + } + + /// + /// Every array-pattern branch of ProcessPatterns already checked for suspension after resolving + /// a member target; the two object-pattern branches did not. + /// + [Fact] + public void AnObjectPatternTargetingAMemberOfAnAwaitedValueRunsItsSideEffectsOnce() + { + AsyncProbe("var o = {}; var p = Promise.resolve(o);", "({ q: (await p).a } = { q: 5 });", "JSON.stringify(o)") + .Should().Be("{\"a\":5}|pre,post"); + } + + [Fact] + public void AnObjectRestTargetingAMemberOfAnAwaitedValueRunsItsSideEffectsOnce() + { + AsyncProbe("var o = {}; var p = Promise.resolve(o);", "({ ...(await p).a } = { q: 5 });", "JSON.stringify(o)") + .Should().Be("{\"a\":{\"q\":5}}|pre,post"); + } + + /// + /// The array-pattern twins of the two above, which carried the check already. Controls. + /// + [Fact] + public void AnArrayPatternTargetingAMemberOfAnAwaitedValueIsAControl() + { + AsyncProbe("var o = {}; var p = Promise.resolve(o);", "[(await p).a] = [5];", "JSON.stringify(o)") + .Should().Be("{\"a\":5}|pre,post"); + } + + [Fact] + public void AnArrayRestTargetingAMemberOfAnAwaitedValueIsAControl() + { + AsyncProbe("var o = {}; var p = Promise.resolve(o);", "[...(await p).a] = [1, 2];", "JSON.stringify(o)") + .Should().Be("{\"a\":[1,2]}|pre,post"); + } + + /// + /// The for-in/for-of head resolves its non-destructuring target through the same + /// Evaluate-then-write shape, in JintForInForOfStatement rather than in an expression. + /// + [Fact] + public void AForOfHeadTargetingAMemberOfAnAwaitedValueRunsItsSideEffectsOnce() + { + AsyncProbe("var o = {}; var p = Promise.resolve(o);", "for ((await p).a of [7]) { }", "JSON.stringify(o)") + .Should().Be("{\"a\":7}|pre,post"); + } + + [Fact] + public void AForInHeadTargetingAMemberOfAnAwaitedValueRunsItsSideEffectsOnce() + { + AsyncProbe("var o = {}; var p = Promise.resolve(o);", "for ((await p).a in { z: 1 }) { }", "JSON.stringify(o)") + .Should().Be("{\"a\":\"z\"}|pre,post"); + } + + [Fact] + public void AForAwaitOfHeadTargetingAMemberOfAnAwaitedValueRunsItsSideEffectsOnce() + { + AsyncProbe("var o = {}; var p = Promise.resolve(o);", "for await ((await p).a of [7]) { }", "JSON.stringify(o)") + .Should().Be("{\"a\":7}|pre,post"); + } + + // ------------------------------------------------------------------ the generator twin + + /// + /// Generators share ISuspendable and its SuspendData with async functions, so they hit + /// the same unguarded lanes — but nothing swallows the TypeError raised while the frame is + /// suspended, so it came straight out of next() instead of turning into a replay. + /// + [Fact] + public void AnOptionalComputedReadOfAYieldedValueCompletes() + { + GeneratorProbe("", "var r = (yield 1)?.[0];", "r", "['zero']").Should().Be("zero|pre,post"); + } + + [Fact] + public void AComputedReadOfAYieldedValueCompletes() + { + GeneratorProbe("", "var r = (yield 1)[0];", "r", "['zero']").Should().Be("zero|pre,post"); + } + + [Fact] + public void AnOptionalReadOfAYieldedValueCompletes() + { + GeneratorProbe("", "var r = (yield 1)?.x;", "r", "{ x: 5 }").Should().Be("5|pre,post"); + } + + [Fact] + public void AnUpdateOfAMemberOfAYieldedValueCompletes() + { + GeneratorProbe("var o = { x: 1 };", "(yield 1).x++;", "o.x", "o").Should().Be("2|pre,post"); + } + + [Fact] + public void TypeOfAMemberOfAYieldedValueCompletes() + { + GeneratorProbe("", "var t = typeof (yield 1).x;", "t", "{ x: 1 }").Should().Be("number|pre,post"); + } + + [Fact] + public void DeletingAMemberOfAYieldedValueCompletes() + { + GeneratorProbe("var o = { x: 1 };", "delete (yield 1).x;", "'x' in o", "o").Should().Be("false|pre,post"); + } + + [Fact] + public void ACompoundAssignmentToAMemberOfAYieldedValueCompletes() + { + GeneratorProbe("var o = { x: 1 };", "(yield 1).x += 1;", "o.x", "o").Should().Be("2|pre,post"); + } + + [Fact] + public void AnAssignmentToAMemberOfAYieldedValueCompletes() + { + GeneratorProbe("var o = { x: 1 };", "(yield 1).x = 9;", "o.x", "o").Should().Be("9|pre,post"); + } + + [Fact] + public void AnObjectPatternTargetingAMemberOfAYieldedValueCompletes() + { + GeneratorProbe("var o = {};", "({ q: (yield 1).a } = { q: 5 });", "JSON.stringify(o)", "o") + .Should().Be("{\"a\":5}|pre,post"); + } + + [Fact] + public void AForOfHeadTargetingAMemberOfAYieldedValueCompletes() + { + GeneratorProbe("var o = {};", "for ((yield 1).a of [7]) { }", "JSON.stringify(o)", "o") + .Should().Be("{\"a\":7}|pre,post"); + } + + /// + /// Plain . on a yielded value takes the guarded fast lane. Control. + /// + [Fact] + public void APlainDotReadOfAYieldedValueIsAControl() + { + GeneratorProbe("", "var r = (yield 1).x;", "r", "{ x: 3 }").Should().Be("3|pre,post"); + } + + /// + /// An async generator suspends through the same machinery again, with the async half's swallowing + /// behaviour, so it replays rather than throws. + /// + [Fact] + public void AComputedReadOfAnAwaitedValueInAnAsyncGeneratorRunsItsSideEffectsOnce() + { + new Engine().Evaluate(""" + var log = []; + async function* ag() { + log.push('pre'); + var v = (await Promise.resolve(['z']))[0]; + log.push('post'); + yield v; + } + ag().next().then(function (r) { return String(r.value) + '|' + log.join(','); }); + """).UnwrapIfPromise().AsString().Should().Be("z|pre,post"); + } +} diff --git a/Jint/Runtime/Interpreter/Expressions/DestructuringPatternAssignmentExpression.cs b/Jint/Runtime/Interpreter/Expressions/DestructuringPatternAssignmentExpression.cs index 9671773f4e..a40f42221b 100644 --- a/Jint/Runtime/Interpreter/Expressions/DestructuringPatternAssignmentExpression.cs +++ b/Jint/Runtime/Interpreter/Expressions/DestructuringPatternAssignmentExpression.cs @@ -631,6 +631,17 @@ private static JsValue HandleObjectPattern( else if (p.Value is MemberExpression memberExpression) { var reference = GetReferenceFromMember(context, memberExpression); + + // Check for suspension after evaluating the member expression, as every array-pattern + // branch above already does: `({ q: (await p).a } = src)` hands back the suspension + // sentinel, and writing through it threw inside the suspended frame - which also meant + // the statement never reached the point where its resume position is recorded, so the + // body replayed from the start (sebastienros/jint#4086). The resume re-runs the pattern. + if (context.IsSuspended()) + { + return JsValue.Undefined; + } + var value = source.Get(sourceKey); AssignToReference(context.Engine, reference, value, environment); } @@ -675,6 +686,14 @@ private static JsValue HandleObjectPattern( else if (restElement.Argument is MemberExpression memberExpression) { var left = GetReferenceFromMember(context, memberExpression); + + // Same suspension check as the property branch above: `({ ...(await p).a } = src)` must + // not copy into the suspension sentinel (sebastienros/jint#4086). + if (context.IsSuspended()) + { + return JsValue.Undefined; + } + var rest = context.Engine.Realm.Intrinsics.Object.ConstructShapeBuilding(); source.CopyDataProperties(rest, processedProperties); AssignToReference(context.Engine, left, rest, environment); diff --git a/Jint/Runtime/Interpreter/Expressions/JintAssignmentExpression.cs b/Jint/Runtime/Interpreter/Expressions/JintAssignmentExpression.cs index d3ad008a6a..07c9da5996 100644 --- a/Jint/Runtime/Interpreter/Expressions/JintAssignmentExpression.cs +++ b/Jint/Runtime/Interpreter/Expressions/JintAssignmentExpression.cs @@ -104,7 +104,20 @@ private JsValue EvaluateMaterialized(EvaluationContext context) else { // fast lookup with binding name failed, we need to go through the reference - lref = (_left.Evaluate(context) as Reference)!; + var leftTarget = _left.Evaluate(context); + + // `(await p).x += 1`: the left-hand side suspended and this is the suspension sentinel. It must + // not be read, and above all must not be parked as the resolved lref below — the resume would + // then compound into Reference(undefined, undefined) (sebastienros/jint#4086). Nothing is + // parked, so the resume re-evaluates the left-hand side; the member link parks its own already + // resolved base, so a side-effecting object expression still runs once. + if (context.IsSuspended()) + { + engine._referencePool.Return(leftTarget as Reference); + return JsValue.Undefined; + } + + lref = (leftTarget as Reference)!; if (lref is null) { Throw.ReferenceError(context.Engine.Realm, "Invalid left-hand side in assignment"); @@ -989,7 +1002,22 @@ private JsValue SetValue(EvaluationContext context) } else { - lref = _left.Evaluate(context) as Reference; + var leftTarget = _left.Evaluate(context); + + // The left-hand side suspended, so this is the suspension sentinel: an unfinished + // Reference(undefined, undefined) for `(await p).x = 1`, or a Reference(base, undefined) + // whose key is still to come for `o[await k] = 1`. Parking either one as the resolved lref + // below is what the parking mechanism must never do — the resume then wrote through it, so + // `(await p).x = 1` rejected with a TypeError and `o[await k] = 1` landed on the literal key + // "undefined" and never on the real one (sebastienros/jint#4086). Bailing here also stops + // the right-hand side from being evaluated on this pass and again on the resume. + if (context.IsSuspended()) + { + engine._referencePool.Return(leftTarget as Reference); + return JsValue.Undefined; + } + + lref = leftTarget as Reference; if (lref is null) { Throw.ReferenceError(engine.Realm, "Invalid left-hand side in assignment"); diff --git a/Jint/Runtime/Interpreter/Expressions/JintMemberExpression.cs b/Jint/Runtime/Interpreter/Expressions/JintMemberExpression.cs index 82f384f25f..9804edbc13 100644 --- a/Jint/Runtime/Interpreter/Expressions/JintMemberExpression.cs +++ b/Jint/Runtime/Interpreter/Expressions/JintMemberExpression.cs @@ -324,9 +324,19 @@ protected override object EvaluateInternal(EvaluationContext context) // The object-side expression itself suspended (e.g. it's a call // expression with an awaiting argument). Do NOT save suspend data: // on resume we re-evaluate _objectExpression so it produces the - // real result via its own resume mechanism. Returning a sentinel - // Reference here matches previous behavior; the caller's IsSuspended - // check bails before use. + // real result via its own resume mechanism. + // + // What comes back is a sentinel Reference(undefined, undefined), and it is only ever a + // placeholder for the shape of the answer: every caller owes an IsSuspended() check + // before it reads one. That used to be asserted here as though it held; it did not, and + // completing the read through Reference(undefined, undefined) raised a TypeError inside + // a frame that was already suspended, which then wiped the statement-list resume + // position on its way out and replayed the whole body (sebastienros/jint#4086). The + // checks are in JintMemberExpression.GetValue (both lanes) and GetCalleeForCall, + // JintCallExpression (callee and argument list), JintUnaryExpression (typeof, delete), + // JintUpdateExpression, JintAssignmentExpression (both forms), + // DestructuringPatternAssignmentExpression and JintTaggedTemplateExpression - a new + // consumer of an evaluated member reference owes one too. return context.Engine._referencePool.Rent(JsValue.Undefined, JsValue.Undefined, strict, thisValue: null); } if (ReferenceEquals(ShortCircuited, baseReference)) @@ -602,6 +612,22 @@ public override JsValue GetValue(EvaluationContext context) } var result = Evaluate(context); + + // Before the `is not Reference` test rather than after it, because a suspended pass can hand back + // either kind and neither may be used. A suspension on the object side produces the sentinel + // Reference(undefined, undefined), and completing that read is what raised a TypeError inside an + // already-suspended frame (sebastienros/jint#4086); one on the property side produces + // Reference(base, undefined), whose completion is an observable `base[undefined]` probe that a + // Proxy or a getter sees. Testing first also keeps a non-Reference signal from escaping as a value. + // Returns Undefined exactly as the guarded fast lane above does; the caller discards it after its + // own IsSuspended() check. + if (context.IsSuspended()) + { + // Resume re-evaluates this node and rents its own reference, so this one is done with. + engine._referencePool.Return(result as Reference); + return JsValue.Undefined; + } + if (result is not Reference reference) { // see JintExpression.GetValue: not a Reference means the protocol guarantees a JsValue diff --git a/Jint/Runtime/Interpreter/Expressions/JintUnaryExpression.cs b/Jint/Runtime/Interpreter/Expressions/JintUnaryExpression.cs index 73312d1a56..6737d58760 100644 --- a/Jint/Runtime/Interpreter/Expressions/JintUnaryExpression.cs +++ b/Jint/Runtime/Interpreter/Expressions/JintUnaryExpression.cs @@ -84,6 +84,16 @@ protected override object EvaluateInternal(EvaluationContext context) { var engine = context.Engine; var result = _argument.Evaluate(context); + + // `typeof (await p).x`: the operand suspended and what came back is the suspension sentinel, + // never a reference to read (sebastienros/jint#4086). The value is discarded by the caller's + // own suspension check; the resume re-evaluates the operand. + if (context.IsSuspended()) + { + engine._referencePool.Return(result as Reference); + return JsValue.Undefined; + } + JsValue v; if (result is Reference rf) @@ -251,7 +261,19 @@ private JsValue EvaluateJsValue(EvaluationContext context) case Operator.Delete: // https://262.ecma-international.org/5.1/#sec-11.4.1 - if (_argument.Evaluate(context) is not Reference r) + var deleteTarget = _argument.Evaluate(context); + + // `delete (await p).x`: the operand suspended, so this is the suspension sentinel and the + // delete has not happened yet — deleting through it coerced undefined to an object and + // threw inside the suspended frame (sebastienros/jint#4086). The resume re-runs the whole + // operator, so answering anything here is fine; the caller discards it. + if (context.IsSuspended()) + { + engine._referencePool.Return(deleteTarget as Reference); + return JsValue.Undefined; + } + + if (deleteTarget is not Reference r) { return JsBoolean.True; } diff --git a/Jint/Runtime/Interpreter/Expressions/JintUpdateExpression.cs b/Jint/Runtime/Interpreter/Expressions/JintUpdateExpression.cs index 8a98377ecb..da607152b9 100644 --- a/Jint/Runtime/Interpreter/Expressions/JintUpdateExpression.cs +++ b/Jint/Runtime/Interpreter/Expressions/JintUpdateExpression.cs @@ -138,7 +138,18 @@ private bool TryUpdateIdentifierNumberSlot(EvaluationContext context, out JsValu private JsValue UpdateNonIdentifier(EvaluationContext context) { var engine = context.Engine; - var reference = _argument.Evaluate(context) as Reference; + var target = _argument.Evaluate(context); + + // `(await p).x++`: the target suspended and this is the suspension sentinel, not a reference to + // read and write back (sebastienros/jint#4086). Bail before the "invalid left-hand side" test as + // well, since a suspended pass is not an invalid target. The resume re-evaluates the whole update. + if (context.IsSuspended()) + { + engine._referencePool.Return(target as Reference); + return JsValue.Undefined; + } + + var reference = target as Reference; if (reference is null) { Throw.ReferenceError(engine.Realm, "Invalid left-hand side in assignment"); diff --git a/Jint/Runtime/Interpreter/JintStatementList.cs b/Jint/Runtime/Interpreter/JintStatementList.cs index af88c77f91..2778a0af6d 100644 --- a/Jint/Runtime/Interpreter/JintStatementList.cs +++ b/Jint/Runtime/Interpreter/JintStatementList.cs @@ -256,10 +256,24 @@ public Completion Execute(EvaluationContext context) /// private bool LeavingOnException(ISuspendable? suspendable, Exception exception) { - // Safe to do from the first pass: nothing between the throw point and this frame reads or writes - // this list's saved position — only this method's own suspension path sets it, and that path - // cannot be running while an exception is in flight through it. - suspendable?.Data.ClearStatementListPosition(this); + // A suspended frame's resume position outlives an exception in flight through it. The premise this + // guard replaces was that the suspension path above and an unwind cannot be live at once; they can, + // because `await`/`yield` suspend by returning plain undefined and whatever the enclosing + // expression then does with that sentinel runs *inside* the suspended frame — so a throw from it + // unwinds through here while the position it just saved is the only record of where to resume. + // Clearing it sent the resume back to statement 0 and replayed the whole body: every un-awaited + // side effect again, and a re-entrancy guard silently truncating the rest (sebastienros/jint#4086). + // Nothing consumes the position but a resume of this same suspended frame, which is exactly the + // case being kept, so keeping it cannot mis-resume some other shape — the normal-completion and + // return-requested paths above already clear it under the same condition. + // + // Still safe to do from the first pass: nothing between the throw point and this frame reads or + // writes this list's saved position. + if (suspendable is not null && !suspendable.IsSuspended) + { + suspendable.Data.ClearStatementListPosition(this); + } + return ShouldCatch(exception); } diff --git a/Jint/Runtime/Interpreter/Statements/JintForInForOfStatement.cs b/Jint/Runtime/Interpreter/Statements/JintForInForOfStatement.cs index cccbd65559..c46ce37b68 100644 --- a/Jint/Runtime/Interpreter/Statements/JintForInForOfStatement.cs +++ b/Jint/Runtime/Interpreter/Statements/JintForInForOfStatement.cs @@ -655,6 +655,25 @@ private Completion BodyEvaluation( if (!destructuring) { lhsRef = lhs!.Evaluate(context); + + // `for ((await p).a of it)`: the target expression itself suspended, so this is + // the suspension sentinel rather than a reference the step's value may be + // written through — doing so threw inside a frame that was already suspended + // (sebastienros/jint#4086). Mirrors the destructuring branch below, including + // its `close = false`: the iterator is not closed, the resume replays the loop. + if (context.IsSuspended()) + { + close = false; + engine._referencePool.Return(lhsRef as Reference); + if (_iterationKind == IterationKind.AsyncIterate && suspendable is not null) + { + var lhsAsyncData = suspendable.Data.GetOrCreate(this); + lhsAsyncData.CurrentValue = valueForResume; + lhsAsyncData.AccumulatedValue = v; + } + completionType = CompletionType.Return; + return new Completion(CompletionType.Return, suspendable?.SuspendedValue ?? nextValue, _statement!); + } } } else if (reusableEnv is not null)