Skip to content

Race conditions on simultaneous hot restarts #1869

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
Closed
annagrin opened this issue Jan 6, 2023 · 2 comments
Closed

Race conditions on simultaneous hot restarts #1869

annagrin opened this issue Jan 6, 2023 · 2 comments
Assignees
@jyameo
Labels
P2 A bug or feature request we're likely to work on package:dwds triaged

Comments

@annagrin
Copy link
Contributor

annagrin commented Jan 6, 2023

Repro:

  • Update dwds dependency in flutter tools to point dwds master
  • run flutter run -d c --web-renderer html --verbose

Then press 'r' a few times (fast). Observe warnings that expression evaluation failed while trying to pre-warm the compiler cache .

This particular failure is benign but indicates that we have a race condition between two createIsolate methods which could put the vm service into a bad state. It could be the cause of flutter/flutter#117676.
annagrin pushed a commit to annagrin/webdev that referenced this issue Jan 6, 2023
Fix race condition on simultaneous hot restarts
0030965 
- Fix race condition on running two `createIsolate` calls simultaneously.
  - Run `destroyIsolate` followed by `createIsolate` an atomic operation.
  - Make debugger API that depend on isolate running wait for the start
    of the app.
  - Make debugger API throw if the isolate exits while the API is waiting.
- Fix exception on uninitialized `DwdsStats._devToolsStart` when using
  an observatory uri to connect to the dwds VM service.
- Remove unnecessary async keywords from synchronoous debugger API.

Closes: ihttps://github.com/dart-lang/issues/1869
@annagrin annagrin mentioned this issue Jan 6, 2023
Merged
annagrin pushed a commit to annagrin/webdev that referenced this issue Jan 6, 2023
Fix race condition on simultaneous hot restarts
f18f0cf 
- Fix race condition on running two `createIsolate` calls simultaneously.
  - Run `destroyIsolate` followed by `createIsolate` an atomic operation.
  - Make debugger API that depend on isolate running wait for the start
    of the app.
  - Make debugger API throw if the isolate exits while the API is waiting.
- Fix exception on uninitialized `DwdsStats._devToolsStart` when using
  an observatory uri to connect to the dwds VM service.
- Remove unnecessary async keywords from synchronoous debugger API.

Closes: ihttps://github.com/dart-lang/issues/1869
annagrin pushed a commit that referenced this issue Jan 10, 2023
Fix race condition on simultaneous hot restarts (#1870)
3743293 
* Wait for isolate to be destroyed before creating a new one

* Update changelog

* Fix race condition on simultaneous hot restarts

- Fix race condition on running two `createIsolate` calls simultaneously.
  - Run `destroyIsolate` followed by `createIsolate` an atomic operation.
  - Make debugger API that depend on isolate running wait for the start
    of the app.
  - Make debugger API throw if the isolate exits while the API is waiting.
- Fix exception on uninitialized `DwdsStats._devToolsStart` when using
  an observatory uri to connect to the dwds VM service.
- Remove unnecessary async keywords from synchronoous debugger API.

Closes: ihttps://github.com//issues/1869

* Fix race condition on simultaneous hot restarts

- Fix race condition on running two `createIsolate` calls simultaneously.
  - Run `destroyIsolate` followed by `createIsolate` an atomic operation.
  - Make debugger API that depend on isolate running wait for the start
    of the app.
  - Make debugger API throw if the isolate exits while the API is waiting.
- Fix exception on uninitialized `DwdsStats._devToolsStart` when using
  an observatory uri to connect to the dwds VM service.
- Remove unnecessary async keywords from synchronoous debugger API.

Closes: ihttps://github.com//issues/1869

* Keep race condition changes only

* Revert unneded changes again

* Merged with master

* Update changelog, pubspec, and build

* Make hot restart atomic changes only

* Address CR Comments
@annagrin annagrin mentioned this issue Jan 10, 2023
Merged
annagrin pushed a commit that referenced this issue Jan 11, 2023
Return error from expression evaluation if the evaluator is closed. (#…
be616cd 
…1884)

* Wait for isolate to be destroyed before creating a new one

* Update changelog

* Fix race condition on simultaneous hot restarts

- Fix race condition on running two `createIsolate` calls simultaneously.
  - Run `destroyIsolate` followed by `createIsolate` an atomic operation.
  - Make debugger API that depend on isolate running wait for the start
    of the app.
  - Make debugger API throw if the isolate exits while the API is waiting.
- Fix exception on uninitialized `DwdsStats._devToolsStart` when using
  an observatory uri to connect to the dwds VM service.
- Remove unnecessary async keywords from synchronoous debugger API.

Closes: ihttps://github.com//issues/1869

* Fix race condition on simultaneous hot restarts

- Fix race condition on running two `createIsolate` calls simultaneously.
  - Run `destroyIsolate` followed by `createIsolate` an atomic operation.
  - Make debugger API that depend on isolate running wait for the start
    of the app.
  - Make debugger API throw if the isolate exits while the API is waiting.
- Fix exception on uninitialized `DwdsStats._devToolsStart` when using
  an observatory uri to connect to the dwds VM service.
- Remove unnecessary async keywords from synchronoous debugger API.

Closes: ihttps://github.com//issues/1869

* Keep race condition changes only

* Revert unneded changes again

* Merged with master

* Update changelog, pubspec, and build

* Error on evaluation if expression evaluator is closed

* Fix test failures

* Remove unnecessary function parsin in FakeInspector
@elliette elliette mentioned this issue Jan 12, 2023
Open
@bkonyi bkonyi added package:dwds P2 A bug or feature request we're likely to work on triaged labels Oct 2, 2024
@bkonyi
Copy link
Collaborator

bkonyi commented Oct 2, 2024

Assigning to @jyameo to verify this is still an issue.

@jyameo
Copy link
Contributor

jyameo commented Oct 24, 2024

I was not able to reproduce this issue. Closing the issue has it appear to have been fixed.

@jyameo jyameo closed this as completed Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jyameo jyameo

Labels
P2 A bug or feature request we're likely to work on package:dwds triaged
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jyameo @annagrin @bkonyi