Repository navigation
Fix hang during shutdown in Flutter workspaces - #6016
Conversation
It appears a recent change to VS Code started waiting up to around 5s for `deactivate()` to complete and showing a "Stopping extension hosts..." notification. It also seems that our shutdown of the Flutter daemon was hanging, but until this change it was not visibile. The issue seems to be that we explicitly called `shutdown()` in deactivate, but VS Code was also calling `dispose()` on everything. The results was that we send a call to `shutdown()` and waited for a response, but VS Code caused the process to be terminated and therefore the response never came and we waited indefinitely. This change causes `shutdown()` to complete if _either_ it gets a response or _or_ the process exits, avoiding this race. Fixes #6015
This comment was marked as outdated.
This comment was marked as outdated.
|
/gemini review |
There was a problem hiding this comment.
Pull request overview
This PR addresses a shutdown hang in Flutter workspaces caused by a race between explicitly calling the Flutter daemon shutdown() during deactivate() and VS Code concurrently disposing/terminating the daemon process. It makes shutdown complete when either the shutdown response arrives or the process exits, preventing VS Code from waiting indefinitely and showing “Stopping extension hosts…”.
Changes:
- Expose a shared
processExitpromise onStdIOServicethat resolves when the underlying process exits. - Update Flutter daemon
shutdown()toawait Promise.race()betweendaemon.shutdownresponse andprocessExit. - Remove the duplicated
ProcessExitCodes/exit tracking from the tooling daemon process and reuse the shared type/export.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/shared/services/tooling_daemon.ts | Switches to the shared ProcessExitCodes export and removes the local exit completer. |
| src/shared/services/stdio_service.ts | Adds processExit promise + exported ProcessExitCodes type to standardize exit tracking across services. |
| src/extension/flutter/flutter_daemon.ts | Prevents shutdown hangs by racing daemon.shutdown against process exit and minor cleanup in dispose(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors process exit handling by moving the processExit promise logic into the StdIOService base class, allowing for broader reuse. The FlutterDaemon.shutdown method was updated to be asynchronous and now uses Promise.race to prevent the shutdown process from hanging. Feedback was provided to maintain the use of curly braces for single-line if statements to ensure style consistency and prevent future errors.
| if (this.pingIntervalId) | ||
| clearInterval(this.pingIntervalId); |
There was a problem hiding this comment.
For consistency and to prevent potential issues when adding more statements, it's a good practice to always use curly braces for if statements, even for single-line blocks. The codebase has an inconsistent style regarding this, and moving towards a consistent style would improve maintainability.
| if (this.pingIntervalId) | |
| clearInterval(this.pingIntervalId); | |
| if (this.pingIntervalId) { | |
| clearInterval(this.pingIntervalId); | |
| } |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6016 +/- ##
==========================================
+ Coverage 68.07% 68.13% +0.05%
==========================================
Files 170 170
Lines 12912 12916 +4
Branches 2572 2573 +1
==========================================
+ Hits 8790 8800 +10
+ Misses 3662 3659 -3
+ Partials 460 457 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
It appears a recent change to VS Code started waiting up to around 5s for
deactivate()to complete and showing a "Stopping extension hosts..." notification.It also seems that our shutdown of the Flutter daemon was hanging, but until this change it was not visibile.
The issue seems to be that we explicitly called
shutdown()in deactivate, but VS Code was also callingdispose()on everything. The results was that we send a call toshutdown()and waited for a response, but VS Code caused the process to be terminated and therefore the response never came and we waited indefinitely.This change causes
shutdown()to complete if either it gets a response or or the process exits, avoiding this race.Fixes #6015