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.

Sign up for GitHub

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

Jump to bottom

[CP] [beta] Cherry pick https://dart-review.googlesource.com/c/sdk/+/328380 #53713

Closed
a-siva opened this issue Oct 9, 2023 · 4 comments
Closed

[CP] [beta] Cherry pick https://dart-review.googlesource.com/c/sdk/+/328380 #53713

a-siva opened this issue Oct 9, 2023 · 4 comments
Assignees
@vsmenon @itsjustkevin @mit-mit @athomas
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. cherry-pick-approved Label for approved cherrypick request cherry-pick-merged Cherry-pick has been merged to the stable or beta branch. cherry-pick-review Issue that need cherry pick triage to approve merge-to-beta

Comments

@a-siva
Copy link
Contributor

a-siva commented Oct 9, 2023 •
edited

Commit(s) to merge

f36c109

Target

beta

Prepared changelist for beta/stable

https://dart-review.googlesource.com/c/sdk/+/329804

Issue Description

Crash with EXC_BAD_ACCESS on ios when profiling.

What is the fix

Don't assume the isolate has a mutator during a profile sample. A profiling sample can be taken when Thread::Current() and Thread::Current()->isolate() are non-NULL. When a thread is entering or exiting an isolate, there is a brief window between the TLS being set/cleared and Isolate::mutator_thread_ being set/cleared

Why cherry-pick

The bug manifests as a crash when users are profiling their apps and hampers developedr productivity.

Risk

low

Issue link(s)

flutter/flutter#134548

Extra Info

No response
@a-siva a-siva added the cherry-pick-review Issue that need cherry pick triage to approve label Oct 9, 2023
@a-siva a-siva added area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. merge-to-beta labels Oct 9, 2023
@a-siva
Copy link
Contributor Author

a-siva commented Oct 12, 2023

@itsjustkevin any updates on this.

@itsjustkevin
Copy link
Contributor

@a-siva it missed the cutoff for this week's release but will be included next week.

Marking as approved as @rmacnak-google has already approved the CL. If there are opinions against, please speak up.

@itsjustkevin itsjustkevin added the cherry-pick-approved Label for approved cherrypick request label Oct 12, 2023
copybara-service bot pushed a commit that referenced this issue Oct 12, 2023
@a-siva
[beta][vm, profiler] Don't assume the isolate has a mutator during a …
6a2fa7a 
…profile sample.

A sample can be taken when Thread::Current() and Thread::Current()->isolate() are non-NULL. When a thread is entering or exiting an isolate, the is a brief window between the TLS being set/cleared and Isolate::mutator_thread_ being set/cleared.

TEST=vm/cc/Profiler_EnterExitIsolate

Fixes: flutter/flutter#134548
Cherry-pick-request: #53713
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/328380
Change-Id: Ic8c0aecd65101a882a8ff6a9ec02c4a9ea1d1cfe
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/329804
Commit-Queue: Siva Annamalai <asiva@google.com>
Reviewed-by: Ryan Macnak <rmacnak@google.com>
@a-siva
Copy link
Contributor Author

a-siva commented Oct 12, 2023

The change has been merged into the beta branch.

@a-siva a-siva closed this as completed Oct 12, 2023
@itsjustkevin
Copy link
Contributor

Reopening: Will close this when it lands in a release

@itsjustkevin itsjustkevin reopened this Oct 12, 2023
@itsjustkevin itsjustkevin added the cherry-pick-merged Cherry-pick has been merged to the stable or beta branch. label Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@vsmenon vsmenon

@itsjustkevin itsjustkevin

@mit-mit mit-mit

@athomas athomas

Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. cherry-pick-approved Label for approved cherrypick request cherry-pick-merged Cherry-pick has been merged to the stable or beta branch. cherry-pick-review Issue that need cherry pick triage to approve merge-to-beta
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@vsmenon @a-siva @itsjustkevin @mit-mit @athomas