-
Notifications
You must be signed in to change notification settings - Fork 82
Defer execution of main until resume for hot restart with DDC library bundle format #2623
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
base: main
Are you sure you want to change the base?
Conversation
… bundle format pause_isolates_on_start tells DWDS and the client that during a hot restart or a hot reload, the VM service will pause and is actively waiting for the client to remove existing breakpoints, reregister them, and then resume. It lets the client know by sending a kPausePostRequest event. In order to do this in DWDS, we need to defer the execution of main until that resume. So, like we do with the require restarter, we wait for a completer to finish before we call main after a hot restart. This completer is only provided when the flag is enabled. Fixes existing code that marks the completer as completed before running main. The previous code canceled the subscription in an unawaited Future. This may result in us recalling main because the event stream could still have a listener. An example is if we hit a breakpoint immediately after main and call resume. Also fixes an issue where metadata information isn't recomputed on a hot restart. This is needed when new files are added. Adds tests for: - Modifying a line with a breakpoint and restarting. - Adding a line before a breakpoint and restarting. - Removing a line before a breakpoint and restarting. - Adding a file and putting a breakpoint in it before restarting.
I believe this should be ready for review - the test failures seem to be tied to existing failures which require investigation. |
@bkonyi Whenever you get a chance, can I have you review this as well? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM overall. It might be worth adding some logic to the tests to verify that the isolates no longer have breakpoints set in libraries that were reloaded or that all breakpoints have been removed after a restart.
…ns of console logs after resume to use a future instead
Thanks! I added logic to check that there are no breakpoints in the isolate after a hot restart but before we process a |
pause_isolates_on_start tells DWDS and the client that during a hot restart or a hot reload, the VM service will pause and is actively waiting for the client to remove existing breakpoints, reregister them, and then resume. It lets the client know by sending a PausePostRequest event.
In order to do this in DWDS, we need to defer the execution of main until that resume. So, like we do with the require restarter, we wait for a completer to finish before we call main after a hot restart. This completer is only provided when the flag is enabled.
Fixes existing code that marks the completer as completed before running main. The previous code canceled the subscription in an unawaited Future. This may result in us recalling main because the event stream could still have a listener. An example is if we hit a breakpoint immediately after main and call resume.
Also fixes an issue where metadata information isn't recomputed on a hot restart. This is needed when new files are added.
Adds tests for: