Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions src/paths/component_iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,14 @@ pub enum MakePathStep<E> {
/// `previous()` (returning `Err(e)` when there is none — i.e. the very first
/// component's parent does not exist).
///
/// Once the walk has advanced forward at all, a subsequent `NotFound` is
/// terminal: the parent was just confirmed `Created`/`Exists`, so stepping
/// back would only bounce between the two forever. This is exactly what
/// happens when a component is a dangling symlink (the link itself is
/// `EEXIST` but any child is `ENOENT`) or a procfs-like path that cannot host
/// children. `node_fs::mkdir_recursive_os_path_impl` encodes the same rule by
/// running a distinct forward pass where `ENOENT` is fatal.
///
Comment thread
robobun marked this conversation as resolved.
Outdated
/// `mkdir` is invoked with `component.path`: a borrowed prefix slice into the
/// original input, never NUL-terminated. Callers that need a sentinel must
/// copy into a scratch buffer.
Expand All @@ -217,14 +225,17 @@ pub fn make_path_with<'a, T: PathChar, E>(
let Some(mut comp) = it.last() else {
return Ok(());
};
let mut advanced = false;
loop {
match mkdir(comp.path)? {
MakePathStep::Created | MakePathStep::Exists => {
advanced = true;
comp = match it.next() {
Some(c) => c,
None => return Ok(()),
};
}
MakePathStep::NotFound(e) if advanced => return Err(e),
Comment thread
robobun marked this conversation as resolved.
MakePathStep::NotFound(e) => {
comp = match it.previous() {
Some(c) => c,
Expand Down Expand Up @@ -389,4 +400,52 @@ mod tests {
assert_eq!(it.next().unwrap().name, b"c");
assert!(it.next().is_none());
}

#[test]
fn make_path_terminates_when_parent_exists_but_child_is_enoent() {
// Model a dangling symlink at `/a/b`: mkdir `/a/b` → EEXIST (the link
// itself exists) but mkdir `/a/b/c` → ENOENT (the link target does
// not). Without the forward-pass termination this loops forever.
Comment thread
robobun marked this conversation as resolved.
Outdated
let it = ComponentIterator::init(&b"/a/b/c"[..], PathFormat::Posix).unwrap();
let mut calls = 0u32;
let r = make_path_with(it, |p| {
calls += 1;
assert!(calls < 100, "runaway loop");
match p {
b"/a" | b"/a/b" => Ok(MakePathStep::Exists),
b"/a/b/c" => Ok(MakePathStep::NotFound(())),
_ => unreachable!(),
}
});
assert!(r.is_err());
// leaf ENOENT → parent EEXIST → leaf ENOENT → stop.
assert_eq!(calls, 3);
}

#[test]
fn make_path_walks_back_then_forward() {
// `/a` exists, `/a/b` and `/a/b/c` do not.
let it = ComponentIterator::init(&b"/a/b/c"[..], PathFormat::Posix).unwrap();
let mut created: Vec<&[u8]> = vec![];
let mut calls = 0u32;
let r = make_path_with::<u8, ()>(it, |p| {
calls += 1;
assert!(calls < 100, "runaway loop");
if p == b"/a" {
return Ok(MakePathStep::Exists);
}
if created.iter().any(|c| *c == p) {
return Ok(MakePathStep::Exists);
}
let parent = &p[..p.iter().rposition(|b| *b == b'/').unwrap()];
if parent == b"/a" || created.iter().any(|c| *c == parent) {
created.push(p);
Ok(MakePathStep::Created)
} else {
Ok(MakePathStep::NotFound(()))
}
});
assert!(r.is_ok());
assert_eq!(created, vec![&b"/a/b"[..], &b"/a/b/c"[..]]);
}
}
48 changes: 48 additions & 0 deletions test/cli/install/bun-install-cache-dir.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { describe, expect, test } from "bun:test";
import { bunEnv, bunExe, isWindows, tempDir } from "harness";
import { symlinkSync } from "node:fs";
import { join } from "node:path";

// A dangling symlink (link itself exists, target does not) makes the
// recursive cache-dir creation loop oscillate: mkdir(link/child) → ENOENT,
// mkdir(link) → EEXIST, repeat. The process spun forever at ~50k mkdirat/s
// with no output. With the fix, the cache-dir open fails and install falls
// back to node_modules/.cache.
//
// Windows is skipped: symlink creation needs Developer Mode / admin, and the
// fix is in the shared path walk so POSIX coverage is sufficient.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
describe.skipIf(isWindows)("BUN_INSTALL_CACHE_DIR inside a dangling symlink", () => {
Comment thread
robobun marked this conversation as resolved.
Outdated
test("bun install exits instead of spinning in mkdirat", { timeout: 60_000 }, async () => {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
using dir = tempDir("cache-dir-dangling", {
"proj/package.json": JSON.stringify({
name: "x",
version: "1.0.0",
dependencies: { "any-pkg": "1.0.0" },
}),
});
const root = String(dir);
symlinkSync(join(root, "does-not-exist"), join(root, "dangling"));

await using proc = Bun.spawn({
cmd: [bunExe(), "install"],
cwd: join(root, "proj"),
env: {
...bunEnv,
BUN_INSTALL_CACHE_DIR: join(root, "dangling", "cache"),
BUN_CONFIG_REGISTRY: "http://127.0.0.1:1/",
},
stdout: "pipe",
stderr: "pipe",
timeout: 15_000,
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

// On hang, Bun.spawn's timeout fires and the child is killed with a
// signal. With the fix, bun exits on its own (registry is unreachable).
expect({ signalCode: proc.signalCode, stdout, stderr }).toMatchObject({
signalCode: null,
});
expect(stderr).toContain("error");
expect(exitCode).not.toBe(0);
});
});
Loading