Skip to content

bun test: scan each directory once, however many symlinks lead to it - #44487

Draft
robobun wants to merge 2 commits into
mainfrom
robobun/d068ecf6/test-scanner-symlinks
Draft

robobun wants to merge 2 commits into
mainfrom
robobun/d068ecf6/test-scanner-symlinks

Conversation

@robobun

@robobun robobun commented Oct 3, 2026

Copy link
Copy Markdown
Collaborator

Problem

  • bun test never exits when two directory symlinks lead back up the tree (ln -s . x && ln -s . y). Memory and open fds grow with no bound: 363 MB and 170,855 fds after 6 s.
  • Scanner::next (src/runtime/cli/test/Scanner.rs) queues a symlinked directory like a real one, so the walk lists a directory once per path to it.

Fix

  • The walk lists each directory once. It skips a link to a directory it reaches without the link, and lists any other linked directory under its real path.
  • Entry::link (src/resolver/fs.rs) returns the kind, a new link bit, and the real path RealFS::kind already reads.
  • Behaviour change: a filter that names only a link path (bun test latest/, latest -> packages/v2) matches nothing now. bun test ./latest still works.
  • Verified: 25 new directory links tests in test/cli/test/bun-test.test.ts. Bun 1.4.3 fails 17.

Background

Downsides

  • A filter or file ignore pattern on a link path misses. A maintainer must accept this, or I add a deferred filter pass.
  • Not measured yet: syscalls, allocations, instructions, binary size. Draft until then.
  • Not fixed: Bun.FileSystemRouter has the same walk (src/router/lib.rs). Two aliasing path roots still record a file twice (bun test: list a test file once when two path arguments select it #43361).
Notes

Question for a maintainer. The walk records a directory that only a link leads to under its real path. A substring filter or a file ignore pattern that names only the link path stops matching. Is that acceptable? If not, a filter pass that runs after the walk, only on trees with links, can keep both names (about 55 more lines). The other way, one listing per link path, brings back the growth this PR removes.

The rule, in the directory arm of Scanner::next, after the existing prunes.

  1. An entry that is not a link, in a directory the walk did not enter through a link: unchanged. No new work.
  2. A link with real path P. If P is the place of the link itself (a Windows reparse point that is not an alias), it is a plain child. If P is the scan root, or is below the root with no pruned name on the way, the walk skips it: the walk lists P by itself. If the walk already followed a link to P, it skips it. Otherwise it records P and lists P once, by absolute real path.
  3. A real child of a directory that the walk entered through a link: the same tests, with P = parent + name.
  4. A path that does not fit a path buffer: not owned, so main's behaviour.

The resolver can hold a listing of a followed directory from before discovery (a parent of the cwd). The read then calls nothing, so the walk replays the cached listing, as it already does for the root.

Numbers: Bun 1.4.3 (367d939) against this branch, same machine. Each line is the Ran N tests across M files result.

Tree 1.4.3 This branch
x -> . and y -> . never exits (3799 files under ulimit -n 64) 1 / 1
examples/one/lib -> ../.. and examples/two/lib -> ../.. never exits 1 / 1
a/loop -> .. and alias -> a never exits 1 / 1
a/loop -> .. 1 / 41 1 / 1
sub/loop -> .. and sub/deeper/self -> . 2 / 902 2 / 2
two packages that link to each other 2 / 82 2 / 2
two links to each of 6 directories in a row 7 / 127 7 / 7
the same with 10 directories 11 / 2047 11 / 11
a link to the parent of the cwd 2 / 81 2 / 2
the root is a link, with a link to its parent inside 2 / 79 2 / 2
root = "tests" in bunfig.toml, tests/up -> .. 2 / 121 2 / 2
a/loop -> .. with --isolate or --parallel=2 82 / 82 2 / 2
shared -> ../outside (only route) 1 / 1 1 / 1
pkg -> .store/pkg (only route) 1 / 1 1 / 1
a link inside a linked directory 2 / 2 2 / 2

Filters and ignore patterns, tree proj/shared -> ../outside, cwd proj.

  • bun test shared/: 1.4.3 runs the test. This branch matches nothing and exits 1.
  • bun test outside/: 1.4.3 matches nothing. This branch runs the test.
  • bun test ./shared: both run the test.
  • --path-ignore-patterns 'shared/**' and 'shared': both prune the link.
  • --path-ignore-patterns 'shared/*.test.js': 1.4.3 ignores the file. This branch runs it.
  • --path-ignore-patterns '../outside/**': 1.4.3 runs the file. This branch ignores it.

The 8 new tests that pass on 1.4.3 too. They pin what must not change: a test that only a link leads to still runs, a filter on the real path still matches, and a link that is the scan root still works. A walk that opens each directory with O_NOFOLLOW cannot reach the directories of the first group.

Why 1.4.0 ended. Before #40016 (in 1.4.1, not in 1.4.0) the walk held one fd for each directory, so it ended when no file descriptor was left.

What I ran. On a debug build of this diff on base bc7a813: test/cli/test/bun-test.test.ts (123 pass, 6 todo, 0 fail), path-ignore-patterns.test.ts (10 pass), test-shard.test.ts (29 pass), pass-with-no-tests.test.ts (5 pass), test/regression/issue/26851.test.ts (2 pass). cargo check for x86_64-pc-windows-msvc and aarch64-apple-darwin, and cargo clippy for bun_resolver and bun_runtime. The branch was then rebased on 519963e with no conflict. I did not run the Windows rows: I have no Windows machine. They use junctions.

Known limits.

  • A directory between the root and a link target that can be searched but not read (chmod 111): the walk skips the link as covered, and cannot list that directory by itself.
  • Remove get_fd_path: derive paths from cwd and what was opened, not from fds #38365 removes get_fd_path and the real path from RealFS::kind. After it, the scanner's own lookup names every link target (open, fd path, close: 3 syscalls per directory link, 0 on a tree with no links), and real_path_of moves to bun_sys::realpath.
  • Other walks with the same defect, not changed here: bust_dir_cache_recursive (src/runtime/api/filesystem_router.rs) and FrameworkRouter::scan_inner (src/runtime/bake/FrameworkRouter.rs).

The reproductions come from #41465. That PR keeps only its other half, the report of a directory that cannot be read.

@robobun

robobun commented Oct 3, 2026

Copy link
Copy Markdown
Collaborator Author

Status

Reproduction on Bun 1.4.3 (Linux x64):

mkdir t && cd t
echo 'import { test } from "bun:test"; test("s", () => {});' > s.test.js
ln -s . x && ln -s . y
bun test   # never exits; memory and open fds grow

With this branch the same tree prints Ran 1 test across 1 file. and exits 0.

This PR is a draft. Two items are open: the cost numbers in Downsides, and the question for a maintainer at the top of Notes.

@robobun

robobun commented Oct 3, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 3:03 AM PT - Oct 3rd, 2026

❌ @robobun, your commit 46db8f9 has 1 failures in Build #123247 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 44487

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

bun-44487 --bun

Comment thread src/resolver/fs.rs
Comment on lines +117 to +118
/// The entry is itself a link (a POSIX symlink, a Windows reparse point).
/// `symlink` alone does not say so: the resolver also fills it for real directories.

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.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

Comment thread src/resolver/fs.rs
Comment on lines +134 to +135
/// Where the link leads, with every link on the way resolved. Empty when
/// the entry is not a link or its target could not be named.

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.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

Comment thread src/resolver/fs.rs
Comment on lines +257 to +260
/// [`Entry::kind`], plus whether the entry is a link and where it leads.
///
/// # Safety
/// Same contract as [`Entry::kind`].

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.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

Comment on lines +222 to +224
// A directory outside the walk's own tree can be one the resolver
// listed before discovery (a parent of the cwd). The read then
// returns that listing and calls nothing.

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.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

Comment on lines +242 to +244
/// Runs `next` over a listing the resolver returned from its cache without
/// calling the iterator (`run_env_loader`/`read_dir_info` read the cwd
/// before the scanner runs).

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.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

Comment on lines +246 to +249
// Hash-map iteration order is not stable. Sort by (lowercased)
// base name so test-file discovery order is deterministic —
// regression/issue/26851 relies on `a_*.test` running before
// `b_*.test` under `--bail`.

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.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

Comment on lines +370 to +372
/// A directory the walk does not enter: a dot-directory, `node_modules`,
/// an excluded name, or a path that an ignore pattern matches. `path`
/// joins to the directory's path and `name_lowercase` is its last part.

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.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

Comment on lines +391 to +393
// reshaped for borrowck — drop the &mut borrow from
// abs_buf and reborrow open_dir_buf immutably so &self methods
// can be called with the slice.

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.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

Comment on lines +409 to +413
/// Decides a directory entry that is a link, or that sits in a directory
/// the walk entered through a link, so that each directory is listed once.
/// A link to a directory the walk reaches by itself is not followed. Any
/// other directory is entered the first time only, under its real path.
/// Returns `true` when the caller queues the entry as a plain child.

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.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

Comment on lines +477 to +478
/// The walk lists `real` by itself: it is the scan root, or it sits below
/// the root and no directory on the way is pruned.

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.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

Comment on lines +623 to +624
/// The part of `path` below `root`, without a leading separator. Empty when
/// both name the same place.

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.

If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code

This branch has not been deployed

No deployments
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.

2 participants