Repository navigation
fix(wsl): close a WSL host channel with stdin EOF before killing wsl.exe - #34307
Conversation
…sl.exe Killing wsl.exe while its guest command still runs can leave the WSL session (an /init with a wslhost.exe helper) behind, which keeps an idle distro running. A closed channel now ends stdin first and kills wsl.exe only if the guest has not exited within 2 s.
|
No actionable comments were generated in the recent review.
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surv… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title "fix(wsl): close a WSL host channel with stdin EOF before killing wsl.exe" matches the main change. The PR closes stdin on wsl.exe first and kills the process only after a 2-second grace per… Description check The description follows the repository template closely. It has ELI5, What Changed, Why, Linked Issue, Visual Proof, Testing, and Checklist sections. It explains the before and after behavior, the mec… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title "fix(wsl): close a WSL host channel with stdin EOF before killing wsl.exe" matches the main change. The PR closes stdin on wsl.exe first and kills the process only after a 2-second grace period. The title is specific and concise, and it follows a conventional commit style. It does not mention the follow-up quit-time kill, but that is a secondary part of the change.
Full details: Description check
-
Explanation
The description follows the repository template closely. It has ELI5, What Changed, Why, Linked Issue, Visual Proof, Testing, and Checklist sections. It explains the before and after behavior, the mechanism (stdin EOF, then kill after WSL_CHANNEL_CLOSE_GRACE_MS of 2 s), and the alternatives considered. It also states that visuals are N/A with a reason. Gaps: the Linked Issue section lists an internal tracker (R40 / E17) and no 'Fixes #' issue, which the template says outside contributors must provide. The template's AI Disclosure, Review, and Agent skill upstream boundary sections are absent. The Testing section lists only the automated tests box, and the manual-testing box is not checked. The lint/typecheck/test/build checklist item is unchecked, with a note that CI covers the rest. The description does not mention the quit-time kill (killWslChannelsInCloseGrace) that the commit messages describe.
Resolution
Consider these improvements before merge: (1) Confirm the Linked Issue is acceptable. The template asks for a 'Fixes #' issue reference, and the description only names an internal tracker. (2) Add a line for the app-quit path (killWslChannelsInCloseGrace), which the objectives describe as a follow-up commit. (3) Complete or explain the AI Disclosure and Agent skill upstream boundary sections. (4) State whether the change was tested manually on a platform, or leave the box unchecked with a reason.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 4 functions across 4 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Review of #34307: a guest command that ignores stdin EOF would otherwise keep its wsl.exe up past Orca's exit for the rest of the grace. Quit teardown now kills every channel still in its grace. The abort test runs on fake timers instead of waiting the grace in real time.
ELI5
When Orca finishes talking to a WSL host over a
wsl.exechannel, it used to killwsl.exeoutright. WSL sometimes leaves that session behind when that happens, and a leftover session keeps the distro, and its virtual machine, running. Now Orca first tells the command it's done (closes its input) and only killswsl.exeif the command hasn't exited within 2 seconds.What Changed
Before:
RemoteCommandChannel.close()on a WSL host channel calledchild.kill()onwsl.exeat once. That covers stdio bridges, liveness watches, and probes that are cancelled or time out. Seen after an idle stop: Orca had nothing left in the distro, yet session/initprocesses with no children stayed, each matched by a lingeringwslhost.exe --distro-id …on Windows. The distro never stopped.After:
close()endswsl.exe's stdin first. Every guest command Orca runs on these channels exits on EOF: the bash relay's stdincat, the Node bridge, and the command loop'sread. So the guest exits by itself,wsl.exeexits normally, and WSL ends the session. If the guest is still running afterWSL_CHANNEL_CLOSE_GRACE_MS(2 s),wsl.exeis killed as before. The timer is unref'd and cleared on exit.Why
What proved it: #33084's real-WSL idle-release phase. A fresh orcad with a 15 s idle timeout; a live terminal must keep the distro up; then the last client leaves and the distro must stop with no release call.
/inits and matchingwslhost.exehelpers.I didn't pin which single channel left each session: on the passing run the failure diagnostics never fired. The fix applies to every close on these channels.
Alternatives: we can't kill WSL's session init from our side, and a longer or shorter grace changes nothing for guests that exit on EOF.
Flag-off: only WSL-host channels use this path. Its single caller is
createWslOrcadHostTransport'sexec, which is used only insidesrc/main/wsl/(WSL host connection, deploy, liveness, pairing repair, decommission), behindORCA_INTERNAL_WSL_HOSTS. With the flag off it never runs. The older WSL bridge (terminals, git, CLI, hook relay) and SSH use other code, so app quit, terminal close and cancelled git operations are untouched.Linked Issue
Internal WSL-via-orcad tracker R40 / E17 (needed by #33084).
Visual Proof
N/A: there's no UI change. This is process teardown inside WSL.
Testing
wsl-command-channel.test.ts(new) checks three things:The WSL transport, stdio bridge, watch and connection suites pass. The real-WSL proof is #33084's lane, which carries this commit until it lands.
Checklist
N/Afor visuals with reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (typecheck, the touched tests and the changed-code gate pass locally; CI runs the rest)