Async and generators: a suspended frame dereferences nothing the suspension produced (backport of #4088) - #4089
Merged
Conversation
…ension produced (backport of sebastienros#4088) Backport of PR sebastienros#4088 (commit bda631c) from main. Closes the 4.x half of sebastienros#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) <noreply@anthropic.com>
This was referenced Sep 19, 2026
PatrickSt1991
pushed a commit
to Apps2Samsung/Apps2Samsung
that referenced
this pull request
Sep 21, 2026
Updated [Jint](https://github.com/sebastienros/jint) from 4.16.2 to 4.16.3. <details> <summary>Release notes</summary> _Sourced from [Jint's releases](https://github.com/sebastienros/jint/releases)._ ## 4.16.3 Jint 4.16.3 is a maintenance release from the `4.x` branch: **correctness and conformance fixes backported from `main`, and nothing that changes an existing API or an existing default.** If you are on 4.16.2 it is a drop-in update — every public signature is the one 4.16.0 shipped, on all five target frameworks, and the per-framework snapshots in `Jint.Tests.PublicInterface/Verify/` are unchanged. `main` remains 5.0.0 development; what is coming there is recorded as it lands in [`docs/v5-migration.md`](https://github.com/sebastienros/jint/blob/main/docs/v5-migration.md). ### Highlights **A long-lived engine stops accumulating what it has already run.** `Evaluate(string)` and `Execute(string)` parse a fresh `Script` on every call, and the engine kept every one of them. Three of the four per-engine handler-tree caches already reset wholesale at 2048 entries so a host streaming endless distinct sources cannot grow them without bound; the fourth, `_evaluatedScripts`, never got that ceiling and held its keys strongly, retaining the AST of every distinct script the engine had ever evaluated — about 528 bytes per call, climbing forever and reclaimed by nothing short of dropping the engine (#4116). The realm's tagged-template map had the same shape and a harder constraint: `Realm._templateMap` was a `Dictionary<Node, JsArray>`, strong on both ends and never cleared, costing roughly 1.35 KB per call for a frozen array and its raw array. A ceiling is no remedy there, because evicting a live template site is script-visible — `f() === f()` must hold for one site — so it becomes a `ConditionalWeakTable<Node, WeakReference<JsArray>>`, weak on both halves (#4119). Both matter most to exactly the embedding that looks innocuous: one engine, kept for the lifetime of the process, handed ad-hoc source. **A suspended frame no longer dereferences what the suspension produced.** `await` and `yield` suspend by returning a plain `undefined`, and the enclosing member link turns that into a sentinel reference 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` swallowed that throw, but not before the statement-list resume position had been cleared on the way out — so the resume replayed the body from the first statement: one extra run of every un-awaited side effect per suspension point, and a re-entrancy guard silently truncating the rest. In a generator nothing swallows it and the `TypeError` comes straight out of `next()`. Two shapes were wrong answers rather than repeated ones — `(await p).x = 1` rejected the promise, and `o[await k] = 1` assigned to the literal key `"undefined"` instead of the real one — and `for await ((await p).a of it)` never terminated at all. Optional chaining was 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 fell through, `(await p)[0]` as much as `(await p)[k]` (#4089, reported by @davidwengier in #4086). ### Verification Every change was verified failing-first against the unfixed branch. The suspension fix is pinned by 35 new cases in `Jint.Tests/Runtime/SuspendedOptionalChainTests.cs`: against 4.16.2's code **28 fail and 6 pass on both .NET 10 and .NET Framework 4.7.2**, with a 35th — the `for await` shape — hanging the test host outright rather than failing; after the fix all 35 pass on both. The retention fixes are pinned by `Jint.Tests/Runtime/GarbageCollectionTests.cs` and `TaggedTemplateCacheTests.cs`. Release diagnostics on the tagged commit, in Release: `Jint.Tests` 7,170 (net10.0) and 7,085 (net472); `Jint.Tests.PublicInterface` 1,852 and 1,844; `Jint.Tests.CommonScripts` 28 and 28; `Jint.Tests.SourceGenerators` 52; the host-contract verification leg (`JINT_HOST_CONTRACT_VERIFICATION=1`) 7,170 / 7,085 and 1,856 / 1,848 — zero failures anywhere. test262: **102,498 passed, 183 skipped**, with three files crossing the engine's default 30-second budget under whole-suite CPU contention and passing in three seconds when run alone. The paired SunSpider and Dromaeo comparison against 4.16.2 was run after the tag rather than before it, which is a departure from how 4.16.2 was gated; it is recorded here because the result is what the release notes should carry, not the order it arrived in. **No row regressed.** Fifty-one rows, paired, alternating order, `DefaultJob`, on an idle machine: the three-round screen left two candidates clearing the sign-agreement and magnitude bar, both of them `Dromaeo.StringBase64`, and re-measuring those at eight rounds read −1.72% [−3.01, +1.98] and +0.30% [−3.10, +4.26] — no change, with a third parameter combination coming out faster. `StringBase64` is the row `Jint.Benchmark/AGENTS.md` already documents as a three-round false positive, and it behaved as documented. The `Cube` control rows moved +0.6% to +0.8%, which is this machine's floor on rows the change cannot reach. Nothing here is a performance change by intent. #4119 does move a tagged-template lookup from a `Dictionary` to a `ConditionalWeakTable` and #4089 adds suspension checks to several interpreter lanes, and neither is visible above the noise floor. ## What's Changed * Backport #4115 to 4.x: bound the evaluated-script set by @lahma in sebastienros/jint#4116 * Backport #4118 to 4.x: stop the realm's template map retaining every site it ran by @lahma in sebastienros/jint#4119 * Async and generators: a suspended frame dereferences nothing the suspension produced (backport of #4088) by @lahma in sebastienros/jint#4089 **Full Changelog**: sebastienros/jint@v4.16.2...v4.16.3 Commits viewable in [compare view](sebastienros/jint@v4.16.2...v4.16.3). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore <dependency name> major version` will close this group update PR and stop Dependabot creating any more for the specific dependency's major version (unless you unignore this specific dependency's major version or upgrade to it yourself) - `@dependabot ignore <dependency name> minor version` will close this group update PR and stop Dependabot creating any more for the specific dependency's minor version (unless you unignore this specific dependency's minor version or upgrade to it yourself) - `@dependabot ignore <dependency name>` will close this group update PR and stop Dependabot creating any more for the specific dependency (unless you unignore this specific dependency or upgrade to it yourself) - `@dependabot unignore <dependency name>` will remove all of the ignore conditions of the specified dependency - `@dependabot unignore <dependency name> <ignore condition>` will remove the ignore condition of the specified dependency and ignore conditions </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.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
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.
Backport of #4088 (
bda631c87) to4.x. The fix landed onmainonly; this is the same fix for the maintenance branch. Refs #4086.What was wrong
awaitandyieldsuspend by returning a plainJsValue.Undefined, and the enclosing member link turns that into a sentinelReference(undefined, undefined)— a placeholder for the shape of the answer, never a reference to read. Every consumer owes acontext.IsSuspended()check before touching one. Nine did not, so they readundefined.undefinedand raised aTypeErrorinside a frame that was already suspended.AsyncBlockStartswallows that throw — the state is stillSuspendedAwait— so it never reached the host and only its side effect survived:JintStatementList.LeavingOnExceptioncleared the statement-list resume position on the way out, the resume replayed the body from statement 0, and every un-awaited side effect ran once more per suspension point. With a re-entrancy guard (if (inited) return;) the replay took the early return and silently truncated the rest of the body. A generator has nothing to swallow it, so the same shapes threw straight out ofnext().Two shapes are wrong answers rather than duplicated side effects, and one never terminates:
(await p).x = 1TypeErroro[await k] = 1"undefined", never the real onefor 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.The fix
The missing
context.IsSuspended()checks, added toJintMemberExpression.GetValue's fall-through lane,typeof,delete,++/--, both assignment forms, the two object-pattern branches ofProcessPatternsand the non-destructuring for-in/for-of head; plus the exception filter no longer clears the resume position while the frame is suspended.The per-site checks are the fix. The filter guard is only the net — a statement that throws never reaches the line that records its position, so keeping the position fixes nothing on its own for a shape that throws. It is kept because no shape was found where a kept position can mis-resume, and it stops any future sentinel leak replaying a body silently.
Adapted for 4.x
AGENTS.mdgotcha and thedocs/guide/migrating-to-v5.mdsection are dropped — neither file exists on this branch, and 4.x's rootAGENTS.mdis still the unsplit monolith.Jint.Testsstill is. The attribute is the only difference; the assertions are AwesomeAssertions on both branches.JintMemberExpressioncarriesmain's identical three-lane structure, only offset — so every hunk applied without conflict and was read back against its 4.x landing context rather than trusted to the merge.Evidence on this branch
Jint.Tests/Runtime/SuspendedOptionalChainTests.cs(35 cases) run against the unfixed 4.x tree:The 35th case,
AForAwaitOfHeadTargetingAMemberOfAnAwaitedValueRunsItsSideEffectsOnce, is excluded from the before counts because it does not fail — it hangs the runner, and the test host had to be killed by PID. That is the same 28-plus-one-hang splitmainmeasured before #4088. The six that pass unfixed are the…IsAControlcases, which bound the claim rather than evidence it.Full 4.x solution, Release:
Jint.Tests7 162 (net10.0) / 7 077 (net472),Jint.Tests.PublicInterface1 852 / 1 844,Jint.Tests.CommonScripts28 / 28,Jint.Tests.SourceGenerators52 — 0 failures anywhere.test262: 102 499 passed, 183 skipped, 2 failed — and the same 2 failed / 102 499 passed / 183 skipped on the unfixed tree in a control run on this machine. Both are
intl402/supportedLocalesOf-unicode-extensions-ignored.js(strict and sloppy) crossing the engine's 30 s timeout at 31 s under whole-suite CPU contention; run in isolation the file passes in 2 s on both trees. Pre-existing on this box, and nowhere near a suspension lane.🤖 Generated with Claude Code