Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CP] Please cherry-pick cc80ccfaa0b66a89a04cd07081d9f42bcf3d2bcf to stable #50415

Closed
annagrin opened this issue Nov 8, 2022 · 8 comments
Closed
Assignees
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. cherry-pick-approved Label for approved cherrypick request merge-to-stable

Comments

@annagrin
Copy link
Contributor

annagrin commented Nov 8, 2022

Commit(s) to merge

cc80ccf

Target

beta, stable

Prepared changelist for beta/stable

https://dart-review.git.corp.google.com/c/sdk/+/268604

Issue Description

Issue

Defining a mixin application in a separate library from the mixin declaration with a private field caused stack overflow during execution.

Platforms

web

What is the fix

The issue was caused by and optimization using this to implement calls to super for non-overriden private accessors.
The fix is to disable the optimization inside the bodies of mixin application accessor stubs.

Why cherry-pick

The issue has caused crashes for users of package:riverpod/riverpod.dart:

Original issue: rrousselGit/riverpod#1713

Risk

low

Issue link(s)

#50119

Extra Info

No response

@annagrin annagrin added the cherry-pick-review Issue that need cherry pick triage to approve label Nov 8, 2022
@itsjustkevin
Copy link
Contributor

@annagrin to solve for creating two cherry-pick issues, we can add the second commit hash to this issue in the commit(s) to merge field separated by a comma.

@itsjustkevin
Copy link
Contributor

itsjustkevin commented Nov 8, 2022

@a-siva @vsmenon @johnniwinther @athomas @devoncarew @sigmundch could you all take a look at this cherry-pick request?

@vsmenon
Copy link
Member

vsmenon commented Nov 8, 2022

lgtm - thanks for the fix, Anna!

@annagrin annagrin changed the title [CP] Please cherry-pick cc80ccfaa0b66a89a04cd07081d9f42bcf3d2bcf to beta and stable [CP] Please cherry-pick cc80ccfaa0b66a89a04cd07081d9f42bcf3d2bcf to stable Nov 9, 2022
@a-siva
Copy link
Contributor

a-siva commented Nov 9, 2022

lgtm

@a-siva a-siva added the area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. label Nov 9, 2022
@sigmundch
Copy link
Member

lgtm

1 similar comment
@johnniwinther
Copy link
Member

lgtm

@itsjustkevin
Copy link
Contributor

Approving.

@itsjustkevin itsjustkevin added cherry-pick-approved Label for approved cherrypick request merge-to-stable and removed cherry-pick-review Issue that need cherry pick triage to approve labels Nov 9, 2022
copybara-service bot pushed a commit that referenced this issue Nov 11, 2022
Generate calls to super class methods from mixin application
stubs using `super` instead of `this`.

Bug: #50119
Change-Id: I02802ea828d72d9221f1886195acb91e6c9520ac
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/267822
Fixed:#50415
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/268604
Reviewed-by: Sigmund Cherem <[email protected]>
Reviewed-by: Joshua Litt <[email protected]>
@whesse
Copy link
Contributor

whesse commented Dec 7, 2022

This fix was landed on stable as

https://dart.googlesource.com/sdk/+/657d2c945c6e5518ace546ff70803155e9b45f31

and was released in stable 2.18.5

@whesse whesse closed this as completed Dec 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. cherry-pick-approved Label for approved cherrypick request merge-to-stable
Projects
None yet
Development

No branches or pull requests

9 participants