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] [Stable][Beta] Please cherry pick 82520abfb1dd82b36e4a50371e71572b2bd95b73 and 84b38aee393d9afc5f2cc83b8a816230478d0646 #54699

Closed
a-siva opened this issue Jan 23, 2024 · 8 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. cherry-pick-approved Label for approved cherrypick request cherry-pick-review Issue that need cherry pick triage to approve merge-to-beta merge-to-stable

Comments

@a-siva
Copy link
Contributor

a-siva commented Jan 23, 2024

Commit(s) to merge

82520ab and 84b38ae

Target

stable and beta

Prepared changelist for beta/stable

stable - https://dart-review.googlesource.com/c/sdk/+/347650
beta - https://dart-review.googlesource.com/c/sdk/+/347760

Issue Description

The change fixes the following

  • an issue that causes Flutter apps to freeze when breakpoints are added to multiple isolates at the same time
  • an issue that causes Flutter apps to crash during hot reload

What is the fix

  • Use safepoint-safe RwLock for breakpoint locations locks
  • Ensure setting breakpoints is lock-safe

Why cherry-pick

Flutter app developers have encountered these scenarios effecting developer productivity.

Risk

low

Issue link(s)

#54650
flutter/flutter#140878

Extra Info

No response

@a-siva a-siva added the cherry-pick-review Issue that need cherry pick triage to approve label Jan 23, 2024
@a-siva
Copy link
Contributor Author

a-siva commented Jan 23, 2024

//cc @aam

@a-siva
Copy link
Contributor Author

a-siva commented Jan 23, 2024

@mit commented with a SGTM on the other issue.

@a-siva a-siva added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Jan 23, 2024
@itsjustkevin itsjustkevin changed the title [CP] [Stable] Please cherry pick 82520abfb1dd82b36e4a50371e71572b2bd95b73 and 84b38aee393d9afc5f2cc83b8a816230478d0646 [CP] [Stable][Beta] Please cherry pick 82520abfb1dd82b36e4a50371e71572b2bd95b73 and 84b38aee393d9afc5f2cc83b8a816230478d0646 Jan 23, 2024
copybara-service bot pushed a commit that referenced this issue Jan 23, 2024
…ions locks and Ensure setting breakpoints is lock-safe.

TEST=DartAPI_BreakpointLockRace and DeoptimizeFramesWhenSettingBreakpoint

Fixes
#54650
flutter/flutter#140878

Acquire reload opreation scope when deoptimizing the world to ensure locks can be acquired for compilation.
Set up scope for operations that can be run while the world is deoptimized and stopped to avoid races.
Ensure code stays unoptizimed when single stepping, prevent other isolates to reoptimize.

Bug: #54650 and flutter/flutter#140878
Change-Id: I9a88096f15a34b645281e5b2b3805a73dd93672e
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/347420 and https://dart-review.googlesource.com/c/sdk/+/345743
Cherry-pick-request: #54699
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/347650
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Kevin Chisholm <kevinjchisholm@google.com>
@a-siva
Copy link
Contributor Author

a-siva commented Jan 23, 2024

@itsjustkevin thanks for taking care of the stable merge, should I go ahead and submit the beta branch CL ?

@itsjustkevin
Copy link
Contributor

Yes @a-siva please push the beta CL also.

copybara-service bot pushed a commit that referenced this issue Jan 23, 2024
…ns locks and Ensure setting breakpoints is lock-safe.

Acquire reload opreation scope when deoptimizing the world to ensure locks can be acquired for compilation.
Set up scope for operations that can be run while the world is deoptimized and stopped to avoid races.
Ensure code stays unoptizimed when single stepping, prevent other isolates to reoptimize it.

Fixes:
#54650
flutter/flutter#140878

TEST=DartAPI_BreakpointLockRace and DeoptimizeFramesWhenSettingBreakpoint

Bug: #54650 and flutter/flutter#140878
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/347420 and https://dart-review.googlesource.com/c/sdk/+/345743
Cherry-pick-request: #54699
Change-Id: Ia4bc883121dac978fbb76027906a810000ef1138
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/347760
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Siva Annamalai <asiva@google.com>
@mellowcello77
Copy link

Just checking how long this fix will take to be available? I've been very eagerly tracking and waiting for a resolution to this issue while I cant debug my projects. Appreciate your efforts on this.

@a-siva
Copy link
Contributor Author

a-siva commented Jan 24, 2024

Just checking how long this fix will take to be available? I've been very eagerly tracking and waiting for a resolution to this issue while I cant debug my projects. Appreciate your efforts on this.

I believe a hotfix release of Flutter is slated for today.

@a-siva
Copy link
Contributor Author

a-siva commented Jan 24, 2024

Yes @a-siva please push the beta CL also.

Done, closing issue.

@DanTup
Copy link
Collaborator

DanTup commented Jan 25, 2024

Thank you! I just verified this on the new stable Flutter and all looks good :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. cherry-pick-approved Label for approved cherrypick request cherry-pick-review Issue that need cherry pick triage to approve merge-to-beta merge-to-stable
Projects
None yet
Development

No branches or pull requests

8 participants