Fix hot restart and breakpoints races #1384
Merged
Conversation
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
grouma
reviewed
Aug 18, 2021
annagrin
force-pushed
the
annagrin/breakpoint_hang
branch
from
de4e24a to
78051a4
Compare
added 6 commits
August 19, 2021 08:55
Multiple breakpoints restored ad the isolate start can cause the app to crash in chrome, serialize them using a mutex to prevent the crash. Hot restart runs main immediately, followed by createIsolate that restores breakpoints, sometimes too late, causing non- removable breakpoints and failures in the UI. Make the dwds run main after createIsolate is done, to make sure breakpoints are all set before the run. Hot restart tries to resume the exiting isolate, and checks if it resumed before that, which causes various races or chrome requiring to be in focus, depending on the way the check is done. Remove the check and just catch exception in case the isolate is already running.
revert timeout on frontend_server_client tests, it is fixed in another PR.
annagrin
force-pushed
the
annagrin/breakpoint_hang
branch
from
036a9dd to
0ad0b99
Compare
Contributor
Author
|
@grouma I removed not running main on hot restart, will have a separate fix for that. |
natebosch
reviewed
Aug 19, 2021
grouma
approved these changes
Aug 20, 2021
nshahan
approved these changes
Aug 20, 2021
Contributor
nshahan
left a comment
nshahan
left a comment
There was a problem hiding this comment.
I'm guessing it's pretty difficult to create a test case that would lead to the "ahh snap" error? Otherwise, LGTM.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Multiple breakpoints restored at the isolate start can cause
the app to crash in chrome, serialize them using a mutex to
prevent the crash.
Hot restart tries to resume the exiting isolate, and checks if
it resumed before that, which causes various races or chrome
requiring to be in focus, depending on the way the check is done.
Remove the check and just catch the exception in case the isolate
is already running.
Closes: #1381
Closes: Dart-Code/Dart-Code#3518