Skip to content

test: wait for the remover to exit in transpiler-cache.test.ts - #41388

Closed
robobun wants to merge 1 commit into
mainfrom
robobun/728fcf38/transpiler-cache-await-remover
Closed

robobun wants to merge 1 commit into
mainfrom
robobun/728fcf38/transpiler-cache-await-remover

Conversation

@robobun

@robobun robobun commented Sep 4, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • test/cli/run/transpiler-cache.test.ts fails on the Windows 11 aarch64 lane. The case after "doing 50 buns at once does not crash" fails in beforeEach (transpiler-cache.test.ts:60): EBUSY: resource busy or locked, rm 'C:\Windows\Temp\buntmp-…\bun.test.…'.
  • The "doing 50 buns" case calls remover.kill(9), but it does not wait for the exit. The remover runs with temp_dir as its cwd. The next beforeEach removes temp_dir about 0.3 ms after the kill.

Fix

  • Declare the remover with await using and killSignal: "SIGKILL". Disposal kills the remover and waits for exited on every path out of the test.
  • This is correct because Windows closes the handles of a process before it signals the process object. exited resolves on that signal.
  • Verified on a Windows 11 arm64 VM, pinned to 4 CPUs (the CI VM size) with 4 busy loops. The old file fails 8 of 8 runs with EBUSY. The new file passes 10 of 10. bun bd test passes on Linux.

Background

  • A Windows process holds an open handle to its current directory. That handle does not allow delete. So the removal of the directory fails with a sharing violation, which libuv reports as EBUSY.
  • kill() calls TerminateProcess, which returns before the process is gone.
  • The race dates from Make tsc --help 2x faster #7365. ci: retry only the test files listed in test/flaky-tests.txt #41204 made it a red build, because CI no longer retries a file that test/flaky-tests.txt does not list. Before that, the same EBUSY passed on a retry in at least 18 builds from 2026-08-28 to 2026-09-02.
Notes
  • Probe on the arm64 VM: spawn a child with a temp dir as its cwd, call kill(9), then rmSync the dir. EBUSY in 30 of 30 runs right after the kill, and in 29 of 30 after one setImmediate. No EBUSY in 30 runs after await proc.exited.
  • Timing of the old file on an idle 16-CPU VM: the next rmSync starts 0.3 ms after the kill, and the test sees the exit at 8.6 ms. The removal of the files in temp_dir usually takes long enough to hide the race.
  • Two cases only (-t "doing 50 buns|disables the cache"), 4 CPUs and 4 busy loops: the old file fails 19 of 20 runs, the new file 0 of 20.
  • CI failures after ci: retry only the test files listed in test/flaky-tests.txt #41204: builds 109855, 109979 and 110213. In 109979 the EBUSY lasted through five beforeEach calls (about 10 ms).
  • The oldest EBUSY for this file that I found is in build 104381 (2026-08-23). I scanned only part of the older builds.
  • A second kill(9) before disposal does not throw on Windows (probe, 40 runs). That is the path where a child fails and onExit kills the remover.
  • bunRun and Bun.spawnSync already wait for the exit. The remover was the only child in the file that the test did not wait for.
  • The race does not occur on Linux or macOS. There, a process does not keep its cwd open against removal.

no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/cli/run/transpiler-cache.test.ts

The "doing 50 buns at once" case killed its remover process, but it did
not wait for the exit. The remover runs with temp_dir as its cwd. On
Windows, a process keeps its cwd open until it exits. So the next
beforeEach could fail to remove temp_dir with EBUSY.

The remover is now declared with `await using`. Disposal kills it and
waits for the exit on every path out of the test.
@robobun

robobun commented Sep 4, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: closed. #45873 landed this fix on main. The closing comment has the details.

Reproduction (Windows 11 arm64 VM, release build of main from 2026-09-04):

  1. Pin the shell to 4 CPUs, the size of the CI VM: [System.Diagnostics.Process]::GetCurrentProcess().ProcessorAffinity = [IntPtr]15
  2. Start 4 busy loops in that shell: bun -e "for(;;){}"
  3. Run bun test test/cli/run/transpiler-cache.test.ts -t "doing 50 buns|disables the cache" 20 times.

The old file fails 19 of 20 runs with EBUSY: resource busy or locked, rm '<temp_dir>' in beforeEach. The file with the fix passes 20 of 20. The race needs Windows, so the Linux lanes cannot show it.

@github-actions github-actions Bot added the claude label Sep 4, 2026
@robobun

robobun commented Sep 4, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 1:31 PM PT - Sep 4th, 2026

❌ @robobun, your commit 54c91ed has 2 failures in Build #110220 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 41388

That installs a local version of the PR into your bun-41388 executable, so you can run:

bun-41388 --bun

@coderabbitai

coderabbitai Bot commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 23de594b-af8a-4690-8b6e-9be901dcfe69

📥 Commits

Reviewing files that changed from the base of the PR and between d296efb and 54c91ed.

📒 Files selected for processing (1)
  • test/cli/run/transpiler-cache.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.


Walkthrough

The transpiler cache test now manages the cache-remover subprocess with scoped asynchronous disposal and SIGKILL. The explicit cleanup kill call was removed, and teardown waits for process exit.

Changes

Transpiler cache cleanup

Layer / File(s) Summary
Scoped subprocess disposal
test/cli/run/transpiler-cache.test.ts
The cache-remover subprocess uses await using with SIGKILL. Its disposer handles termination before temporary-directory cleanup, while the test retains process exit assertions.

Suggested reviewers: jarred-sumner

Merge Risk: ⚪ Minimal · up to 54c91

The transpiler-cache test now waits for its remover subprocess to exit before temporary-directory cleanup, addressing Windows cleanup races. No merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Description check ✅ Passed The description clearly explains the Windows EBUSY failure, the await using fix, the verification results, and relevant background. It does not use the exact template headings, but it provides the req…
Title check ✅ Passed The title accurately and concisely describes the main change: waiting for the remover process to exit in transpiler-cache.test.ts.

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

@robobun

robobun commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator Author

The automated review has no findings, so this commit does not change. The Windows 11 aarch64 test jobs in build 110220 run the changed file. They have not started yet.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM — test-only flake fix that swaps a fire-and-forget kill(9) for await using + killSignal: "SIGKILL" so the remover has actually exited before the next beforeEach deletes its cwd on Windows.

What was reviewed:

  • Confirmed await using on Bun.spawn disposes by killing (with killSignal) and awaiting exited, so the EBUSY window closes deterministically.
  • Checked the surviving remover.kill(9) in the onExit failure branch — a second kill before disposal is a no-op, and disposal still awaits the exit.
  • The added comment is a load-bearing why-comment for the Windows cwd-handle behavior; matches the repo's await using convention for spawned helpers.
Extended reasoning...

Overview

This PR touches a single test file, test/cli/run/transpiler-cache.test.ts, changing one test case ("doing 50 buns at once does not crash"). The remover subprocess is now declared with await using and killSignal: "SIGKILL" instead of a bare const + manual remover.kill(9) mid-test. A three-line comment explains the Windows-specific motivation: a process holds its cwd open until it exits, and the next beforeEach removes that directory. No production code is touched.

Security risks

None. This is a test-infrastructure change with no effect on shipped behavior. It spawns the same helper with the same arguments; only the cleanup ordering changes.

Level of scrutiny

Low. The change is small (net +4/−2), test-only, and mechanically applies the repo's own preferred pattern: await using on Bun.spawn for guaranteed cleanup, cleanup registered before assertions, and awaiting the actual condition (process exit) rather than assuming kill() completed synchronously. The root cause is clearly identified (Windows TerminateProcess returns before the process is gone; cwd handle blocks directory deletion), and the fix addresses exactly that mechanism rather than papering over it with a retry or sleep.

Other factors

The onExit failure path still calls remover.kill(9) explicitly (line 165). That's fine: it's an early kill on child failure, and the await using disposal will still await exited afterward — a second kill on an already-terminating process is a no-op. No CODEOWNERS entry covers this file. The exit reason was dry_streak, no findings were reported, and the PR conversation has no outstanding objections.

@robobun

robobun commented Oct 22, 2026

Copy link
Copy Markdown
Collaborator Author

Closing: #45873 landed the fix for this test.

  • On main, the test declares the remover with await using and no longer calls remover.kill(9). Disposal kills the remover and waits for its exit. So beforeEach cannot remove temp_dir while the remover is alive.
  • Unlike this PR, main does not set killSignal: "SIGKILL", so disposal sends SIGTERM. This leaves no part of the race. On Windows, libuv calls TerminateProcess for SIGTERM and for SIGKILL (uv__kill in src/win/process.c).
  • test: fix 20 flaky tests #45873 reports these numbers from Windows 11 x64. With the old file, 99 of 120 runs fail. With the fix, 0 of 120 fail.
  • CI shows the same. None of the 1716 builds created since the merge (until 2026-10-22) reports this EBUSY. Before the merge, 3 of 132 builds in a 5.5 hour window on 2026-10-17 reported it (127609, 127674, 127706).

A note on build 127706, which #45857 lists as not explained: it is the same race. The job log shows 13 failed cases. The first is the case after "doing 50 buns at once does not crash". The annotation shows only the last 9.

@robobun robobun closed this Oct 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant