Async and generators: a suspended frame dereferences nothing the suspension produced (backport of #4088)

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);

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");

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

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;
                 }

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");

Jint/Runtime/Interpreter/JintStatementList.cs

@@ -256,10 +256,24 @@ public Completion Execute(EvaluationContext context)
     /// </summary>
     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);
     }
 

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<ForAwaitSuspendData>(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)