Skip to content

fix: pass previousRealm to AsyncGeneratorCompleteStep in AsyncGeneratorYield#5106

Open
xcb3d wants to merge 7 commits intoboa-dev:mainfrom
xcb3d:fix/async-generator-yield-previous-realm
Open

fix: pass previousRealm to AsyncGeneratorCompleteStep in AsyncGeneratorYield#5106
xcb3d wants to merge 7 commits intoboa-dev:mainfrom
xcb3d:fix/async-generator-yield-previous-realm

Conversation

@xcb3d
Copy link
Contributor

@xcb3d xcb3d commented Mar 16, 2026

This Pull Request closes #N/A (no existing issue — this is a spec compliance improvement discovered through code audit).

It changes the following:

  • Implements ECMAScript spec §27.6.3.8 AsyncGeneratorYield steps 6-8 by capturing the caller's realm (previousRealm) and passing it to AsyncGeneratorCompleteStep via complete_step().
  • Adds caller_realm: Option<Realm> to CallFrame to carry the caller's realm through the generator resume/yield cycle.
  • Captures context.realm().clone() in GeneratorContext::resume() before the stack swap, storing it in the generator's CallFrame.
  • Passes the captured previous_realm to AsyncGenerator::complete_step() in the AsyncGeneratorYield opcode instead of always passing None.
  • Annotates steps 12b-12c as implicitly handled by Boa's stack-swap architecture (replacing TODO comments with explanatory notes).
  • Adds a cross-realm async generator test that exercises yielding values across different realms.

Background:

Previously, AsyncGeneratorYield always passed None as the realm argument to complete_step(), causing iterator result objects ({value, done}) to always be created in the generator's own realm. Per the spec, they should be created in previousRealm — the realm of the execution context that was active before the generator resumed. The complete_step() function already handled the Some(realm) case correctly (switching realms via enter_realm before calling create_iter_result_object), but was never given a realm to switch to.

This fix captures the caller's realm at the correct point (in GeneratorContext::resume(), before the stack swap) and threads it through the CallFrame so the AsyncGeneratorYield opcode can read it and pass it to complete_step().

@xcb3d xcb3d requested a review from a team as a code owner March 16, 2026 05:30
@github-actions github-actions bot added Waiting On Review Waiting on reviews from the maintainers C-Tests Issues and PRs related to the tests. C-Builtins PRs and Issues related to builtins/intrinsics C-VM Issues and PRs related to the Boa Virtual Machine. and removed Waiting On Review Waiting on reviews from the maintainers labels Mar 16, 2026
@github-actions github-actions bot added this to the v1.0.0 milestone Mar 16, 2026
@xcb3d xcb3d force-pushed the fix/async-generator-yield-previous-realm branch from 28155e6 to df7b5a7 Compare March 16, 2026 05:35
@github-actions github-actions bot added the Waiting On Review Waiting on reviews from the maintainers label Mar 16, 2026
@github-actions
Copy link

github-actions bot commented Mar 16, 2026

Test262 conformance changes

Test result main count PR count difference
Total 52,963 52,963 0
Passed 50,073 50,073 0
Ignored 2,072 2,072 0
Failed 818 818 0
Panics 0 0 0
Conformance 94.54% 94.54% 0.00%

Tested main commit: 197b736bb87523a14b41834c078adbec3059d237
Tested PR commit: c37288f374ecee549ad7e1befb6546851f73745e
Compare commits: 197b736...c37288f

@codecov
Copy link

codecov bot commented Mar 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.31%. Comparing base (6ddc2b4) to head (c37288f).
⚠️ Report is 874 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #5106       +/-   ##
===========================================
+ Coverage   47.24%   59.31%   +12.06%     
===========================================
  Files         476      563       +87     
  Lines       46892    62800    +15908     
===========================================
+ Hits        22154    37248    +15094     
- Misses      24738    25552      +814     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@xcb3d xcb3d requested a review from jedel1043 March 16, 2026 06:21
}

#[test]
fn cross_realm_async_generator_yield() {
Copy link
Member

@jedel1043 jedel1043 Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You also might want to test if the generator is really using the caller's realm to create the iter object. A simple way to do this would be to check that the prototype of the async generator's returned object is equal to the Object.prototype in old_realm.

@jedel1043 jedel1043 added Waiting On Author Waiting on PR changes from the author and removed Waiting On Review Waiting on reviews from the maintainers labels Mar 16, 2026
@xcb3d
Copy link
Contributor Author

xcb3d commented Mar 17, 2026

Updated the previousRealm implementation based on your feedback.

The key challenge was that Boa's native function mechanism doesn't use a spec-compliant execution context stack, so "second to top element" doesn't map directly. Here's the approach:

  1. native_caller_realm on Vmnative_function_call captures the pre-swap_realm caller realm. It's saved/restored (not set/cleared) to handle nested native calls correctly.

  2. caller_realm on CallFrameGeneratorContext::resume() transfers native_caller_realm onto the generator's call frame. It's only set if not already present, because yield expr compiles to AsyncGeneratorYield(Await(expr)) — the Await causes a second resume() via microtask that must not overwrite the correct realm captured on the first resume.

  3. Clear after useAsyncGeneratorYield clears caller_realm after reading it, so the next next() call from a potentially different realm gets a fresh value.

  4. complete_step realm swap — Temporarily enters the caller's realm to create the iterator result object, ensuring its prototype comes from the correct realm.

The cross_realm_async_generator_yield test now passes — it creates a generator in one realm, consumes it from another, and asserts that each yielded iterator result's [[Prototype]] is the caller's Object.prototype, not the generator's.

@github-actions github-actions bot added the Waiting On Review Waiting on reviews from the maintainers label Mar 17, 2026
@xcb3d xcb3d requested a review from jedel1043 March 17, 2026 04:54
Comment on lines +92 to +96
/// The caller's realm before a native function's `swap_realm`.
/// Used by `GeneratorContext::resume()` to capture `previousRealm`
/// for `AsyncGeneratorYield`.
pub(crate) native_caller_realm: Option<Realm>,

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You added this again...

@jedel1043
Copy link
Member

The key challenge was that Boa's native function mechanism doesn't use a spec-compliant execution context stack, so "second to top element" doesn't map directly.

If this is the case, it's a bug. We shouldn't hack around bugs.

@jedel1043 jedel1043 removed the Waiting On Review Waiting on reviews from the maintainers label Mar 17, 2026
@github-actions github-actions bot added the Waiting On Review Waiting on reviews from the maintainers label Mar 17, 2026
@xcb3d
Copy link
Contributor Author

xcb3d commented Mar 17, 2026

Hi @jedel1043, reverted to the frames[len-2].realm approach as suggested — removed native_caller_realm and caller_realm fields.

One question: you mentioned checking the prototype against old_realm's Object.prototype, but frames[len-2].realm at yield time returns the generator's realm (since both next() and the AwaitFulfilled handler have [[Realm]] = generator's realm). Should the test assert against old_realm or generator_realm?

@jedel1043
Copy link
Member

Then complete_step has a bug, and it's not setting the realm correctly on the promise's wrapped iter result

@xcb3d
Copy link
Contributor Author

xcb3d commented Mar 17, 2026

Thanks for the clarification! You're right — frames[len-2].realm currently returns the generator's realm because native_function_call swaps the existing frame's realm instead of pushing a new frame. So the frame below the generator context doesn't reflect the actual caller's realm.

Would you like me to fix complete_step to correctly resolve previousRealm to the caller's realm, or is this something that needs a broader fix in how native function calls interact with the frame stack?

@xcb3d
Copy link
Contributor Author

xcb3d commented Mar 17, 2026

I see two possible approaches to fix the previousRealm resolution:

  1. Make native_function_call push a lightweight frame so the frame stack becomes spec-compliant
  2. Fix complete_step to resolve previousRealm from the promise capability's realm instead of the frame stack

What do you think? Do you have a recommendation on which direction to go?

@xcb3d
Copy link
Contributor Author

xcb3d commented Mar 18, 2026

Hi @jedel1043, can you check my comment pls ?. Thanks

@jedel1043
Copy link
Member

Lightweight frame seems like a good solution

@jedel1043 jedel1043 removed the Waiting On Review Waiting on reviews from the maintainers label Mar 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-Builtins PRs and Issues related to builtins/intrinsics C-Tests Issues and PRs related to the tests. C-VM Issues and PRs related to the Boa Virtual Machine. Waiting On Author Waiting on PR changes from the author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants