Skip to content

fix(uninstall): spawn windows cleanup helper via cmd start trampoline#248

Merged
BlackHole1 merged 1 commit into
mainfrom
fix-win-uninstall
May 30, 2026
Merged

fix(uninstall): spawn windows cleanup helper via cmd start trampoline#248
BlackHole1 merged 1 commit into
mainfrom
fix-win-uninstall

Conversation

@BlackHole1
Copy link
Copy Markdown
Member

On Windows, oo uninstall left the running executable behind. The post-exit cleanup helper was launched with Bun.spawn({ detached: true })

  • unref(), but on Windows Bun places spawned children in a kill-on-close job object and detached (UV_PROCESS_DETACHED) does not break the child out of it. The helper was killed the instant the main process exited — before it could unlink the running image — leaving oo.exe on disk.

Launch the helper through a cmd /c start "" /b trampoline instead, which creates it as a process that breaks away from the job object and outlives the parent. Await the bootstrap cmd (rather than detached + unref) so start has launched the helper before we exit, closing the race where the job could close and kill cmd before start ran. The generated self-delete script is unchanged.

Update the Windows unit assertion for the new command shape, and add a real (non-mock) win32 integration test: a child process runs performSelfUninstall, exits, and the deferred executable must then be removed by the helper — covering the real spawn path that was previously exercised only through a mock.

The job-object behavior reproduces on bun 1.3.14 and 1.4.0-canary.1 (the Rust rewrite), so this is a deliberate long-term workaround rather than a version-specific patch.

Repro: https://github.com/BlackHole1/bun-detached-windows-bug
Issue: oven-sh/bun#31603

On Windows, `oo uninstall` left the running executable behind. The
post-exit cleanup helper was launched with `Bun.spawn({ detached: true })`
+ `unref()`, but on Windows Bun places spawned children in a kill-on-close
job object and `detached` (UV_PROCESS_DETACHED) does not break the child
out of it. The helper was killed the instant the main process exited —
before it could unlink the running image — leaving `oo.exe` on disk.

Launch the helper through a `cmd /c start "" /b` trampoline instead, which
creates it as a process that breaks away from the job object and outlives
the parent. Await the bootstrap `cmd` (rather than detached + unref) so
`start` has launched the helper before we exit, closing the race where the
job could close and kill `cmd` before `start` ran. The generated
self-delete script is unchanged.

Update the Windows unit assertion for the new command shape, and add a
real (non-mock) win32 integration test: a child process runs
`performSelfUninstall`, exits, and the deferred executable must then be
removed by the helper — covering the real spawn path that was previously
exercised only through a mock.

The job-object behavior reproduces on bun 1.3.14 and 1.4.0-canary.1 (the
Rust rewrite), so this is a deliberate long-term workaround rather than a
version-specific patch.

Repro: https://github.com/BlackHole1/bun-detached-windows-bug

Signed-off-by: Kevin Cui <bh@bugs.cc>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 30, 2026

Review Change Stack

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Improved Windows uninstall process to ensure deferred cleanup completes reliably after application exit.
  • Tests

    • Added integration test for Windows uninstall to verify cleanup behavior.

Walkthrough

This PR refactors the Windows self-uninstall helper spawning mechanism. The implementation in spawnWindowsCleanupHelper now uses a cmd /c start "" /b trampoline to launch the PowerShell uninstall helper, replacing the prior Bun detached: true and unref() approach. The new approach spawns cmd via Bun.spawn and awaits subprocess.exited to ensure the helper is actually spawned before returning. Unit test assertions were updated to verify the command structure includes the trampoline and powershell. A new Windows-only integration test was added that validates the deferred executable is removed by the helper after the parent process exits, using a polling waitForMissing helper function.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title follows the required format <type>(<scope>): <subject> with type 'fix', scope 'uninstall', and a clear subject describing the main change of spawning the Windows cleanup helper via cmd start trampoline.
Description check ✅ Passed The description provides comprehensive context about the Windows uninstall bug, explains the root cause (Bun's kill-on-close job object behavior), details the solution (cmd start trampoline), and references the associated tests and upstream issue.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch fix-win-uninstall

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/application/self-update/uninstall.ts`:
- Around line 498-511: The current awaiting of the Bun.spawn trampoline uses
await subprocess.exited but doesn't check the returned exit code; capture the
resolved exit code from subprocess.exited (e.g., const exitCode = await
subprocess.exited) and if it's non-zero throw an Error with a clear message that
includes the exit code and context (referencing the subprocess variable and the
Bun.spawn call that invoked the Windows bootstrap) so failures of the `cmd /c
start ...` bootstrap fail fast instead of being treated as success.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e1efa7da-433d-45be-adeb-1f4160992713

📥 Commits

Reviewing files that changed from the base of the PR and between 8d78cb4 and 43290bd.

📒 Files selected for processing (2)
  • src/application/self-update/uninstall.test.ts
  • src/application/self-update/uninstall.ts

Comment thread src/application/self-update/uninstall.ts
@BlackHole1 BlackHole1 merged commit 12022f8 into main May 30, 2026
6 checks passed
@BlackHole1 BlackHole1 deleted the fix-win-uninstall branch May 30, 2026 02:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant