Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
29 changes: 25 additions & 4 deletions src/runtime/node/node_fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2132,6 +2132,13 @@ mod _async_tasks {

pub(crate) subtask_count: AtomicUsize,

/// Set once `pending_err` is. From then on `enqueue` schedules nothing
/// and a subtask that starts skips its directory, so the scan settles as
/// soon as the subtasks already in flight return instead of draining the
/// whole frontier. Two directory symlinks back to the same tree make
/// that frontier 2^41 paths deep before the kernel reports ELOOP.
pub(crate) has_error: AtomicBool,

/// The final result list
pub(crate) result_list: ResultListEntryValue,

Expand Down Expand Up @@ -2292,6 +2299,9 @@ mod _async_tasks {

impl AsyncReaddirRecursiveTask {
pub(crate) fn enqueue(&mut self, basename: &ZStr) {
if self.has_error.load(Ordering::Relaxed) {
return;
}
// The subtask runs on another thread after the caller's `name_to_copy_z`
// (which points into a per-iteration buffer) has been overwritten, so we
// must heap-own the bytes here. Freed in ReaddirSubtask::call's cleanup.
Expand Down Expand Up @@ -2351,6 +2361,7 @@ mod _async_tasks {
done: None,
has_result: AtomicBool::new(false),
subtask_count: AtomicUsize::new(1),
has_error: AtomicBool::new(false),
root_path,
result_list,
result_list_count: AtomicUsize::new(0),
Expand All @@ -2370,6 +2381,10 @@ mod _async_tasks {
buf: &mut PathBuffer,
is_root: bool,
) {
if self.has_error.load(Ordering::Relaxed) {
self.on_subtask_done();
return;
}
macro_rules! impl_tag {
($T:ty, $variant:ident) => {{
// A bare `Vec::new()` here
Expand Down Expand Up @@ -2401,9 +2416,8 @@ mod _async_tasks {
self.pending_err = Some(err.with_path(err_path));
}
}
if self.subtask_count.fetch_sub(1, Ordering::Relaxed) == 1 {
self.finish_concurrently();
}
self.has_error.store(true, Ordering::Relaxed);
self.on_subtask_done();
}
Ok(()) => {
self.write_results::<$T>(&mut entries);
Expand Down Expand Up @@ -2439,7 +2453,14 @@ mod _async_tasks {
};
}

if self.subtask_count.fetch_sub(1, Ordering::Relaxed) == 1 {
self.on_subtask_done();
}

/// Drops this subtask's `subtask_count` reference. The last one finishes
/// the scan; `AcqRel` publishes every subtask's `pending_err` and queued
/// results to it.
fn on_subtask_done(&mut self) {
if self.subtask_count.fetch_sub(1, Ordering::AcqRel) == 1 {
self.finish_concurrently();
}
}
Expand Down
49 changes: 49 additions & 0 deletions test/js/node/fs/fs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1834,6 +1834,55 @@ it.skipIf(isWindows)("promises.readdir({recursive: true}) settles when multiple
});
});

// After the first subtask failed, the walker kept scheduling a subtask for every
// directory it found and only settled once that frontier drained. Two symlinks
// back to the root make the frontier 2^41 directories (the kernel follows 40
// symlinks before ELOOP), so the promise and the callback never settled.
it.skipIf(isWindows)(
"readdir({recursive: true}) settles with the first error while symlink loops are still being walked",
async () => {
using dir = tempDir("readdir-recursive-loop", {
"a/b/keep.txt": "x",
});
const root = String(dir);
// Opening this one with O_DIRECTORY fails with ELOOP at once.
fs.symlinkSync(join(root, "bad"), join(root, "bad"));
fs.symlinkSync(".", join(root, "loop1"));
fs.symlinkSync(".", join(root, "loop2"));

await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`
const fs = require("fs");
const root = ${JSON.stringify(root)};
const viaPromise = await fs.promises.readdir(root, { recursive: true }).then(
r => "resolved " + r.length,
e => "rejected " + e.code,
);
console.log("promise", viaPromise);
const { promise, resolve } = Promise.withResolvers();
fs.readdir(root, { recursive: true }, (e, r) => resolve(e ? "rejected " + e.code : "resolved " + r.length));
console.log("callback", await promise);
`,
],
env: bunEnv,
stdout: "pipe",
stderr: "inherit",
timeout: 10_000,
});

const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]);

expect({ stdout: stdout.trim(), exitCode, signalCode: proc.signalCode }).toEqual({
stdout: "promise rejected ELOOP\ncallback rejected ELOOP",
exitCode: 0,
signalCode: null,
});
},
);

describe("readSync", () => {
it("rejects the read when the length argument detaches the destination buffer during coercion", () => {
const fd = openSync(import.meta.dir + "/readFileSync.txt", "r");
Expand Down
Loading