node:fs: recursive rm keeps walking when a concurrent deleter wins the race

src/runtime/node/node_fs.rs

@@ -9596,18 +9596,9 @@ fn dt_err(errno: E) -> crate::Error {
 
 #[inline]
 fn dt_open_dir(parent: &sys::Dir, name: &[u8]) -> Result<sys::Dir, E> {
-    let mut path_buf = PathBuffer::uninit();
-    let len = name.len().min(path_buf.len() - 1);
-    path_buf[..len].copy_from_slice(&name[..len]);
-    path_buf[len] = 0;
-    // SAFETY: NUL written at [len].
-    let z = ZStr::from_buf(&path_buf[..], len);
-    match Syscall::openat(
-        parent.fd,
-        z,
-        sys::O::DIRECTORY | sys::O::RDONLY | sys::O::NOFOLLOW,
-        0,
-    ) {
+    // On Windows a delete-pending directory reports ENOENT, like any other
+    // vanished entry (see `openat_dir_for_delete_tree`).
+    match sys::openat_dir_for_delete_tree(parent.fd, name) {
         Ok(fd) => Ok(sys::Dir::from_fd(fd)),
         Err(e) => Err(e.get_errno()),
     }
@@ -9642,10 +9633,12 @@ fn dt_delete_file(parent: &sys::Dir, name: &[u8]) -> Result<(), E> {
             if matches!(errno, E::EPERM | E::EACCES) {
                 // No-follow stat — don't follow symlinks, to match unlinkat.
                 // `z` (a `&ZStr`, `Copy`) is still valid — `unlinkat` only borrowed it.
-                if let Ok(st) = Syscall::lstatat(parent.fd, z) {
-                    if sys::S::ISDIR(st.st_mode as u32) {
-                        return Err(E::EISDIR);
-                    }
+                match Syscall::lstatat(parent.fd, z) {
+                    Ok(st) if sys::S::ISDIR(st.st_mode as u32) => return Err(E::EISDIR),
+                    // The entry vanished between unlinkat and lstat: a
+                    // concurrent deleter won, so the real answer is ENOENT.
+                    Err(e) if e.get_errno() == E::ENOENT => return Err(E::ENOENT),
+                    _ => {}
                 }
             }
             Err(errno)
@@ -9721,6 +9714,10 @@ pub(crate) fn zig_delete_tree(
             let entry = match stack[top_idx].iter.next() {
                 Ok(Some(e)) => e,
                 Ok(None) => break,
+                // A concurrent deleter removed the directory we iterate:
+                // getdents on a dead dir reports ENOENT. Nothing is left to
+                // enumerate, and the rmdir below tolerates ENOENT.
+                Err(err) if err.get_errno() == E::ENOENT => break,
                 Err(err) => return Err(dt_err(err.get_errno())),
             };
             // `entry.name` borrows the iterator's internal buffer and
@@ -9749,6 +9746,9 @@ pub(crate) fn zig_delete_tree(
                                 treat_as_dir = false;
                                 continue 'handle_entry;
                             }
+                            // The entry vanished between readdir and open: a
+                            // concurrent deleter won. Skip it.
+                            Err(E::ENOENT) => break 'handle_entry,
                             #[cfg(target_os = "macos")]
                             Err(e @ (E::EACCES | E::EPERM)) => {
                                 // Same as the pop-delete site below: node's rimraf
@@ -9775,17 +9775,24 @@ pub(crate) fn zig_delete_tree(
                         }
                     } else {
                         let top_fd = stack[top_idx].iter.iter.dir;
-                        zig_delete_tree_min_stack_size_with_kind_hint(
+                        match zig_delete_tree_min_stack_size_with_kind_hint(
                             sys::Dir::borrow(&top_fd),
                             &entry_name,
                             entry.kind,
-                        )?;
-                        break 'handle_entry;
+                        ) {
+                            Ok(()) => break 'handle_entry,
+                            // The entry vanished: skip it. FileNotFound only
+                            // matters to the top-level caller, not mid-tree.
+                            Err(crate::Error::FileNotFound) => break 'handle_entry,
+                            Err(e) => return Err(e),
+                        }
                     }
                 } else {
                     let top_fd = stack[top_idx].iter.iter.dir;
                     match dt_delete_file(sys::Dir::borrow(&top_fd), &entry_name) {
                         Ok(()) => break 'handle_entry,
+                        // A concurrent deleter already unlinked this entry.
+                        Err(E::ENOENT) => break 'handle_entry,
                         Err(E::EISDIR) => {
                             treat_as_dir = true;
                             continue 'handle_entry;
@@ -9984,6 +9991,8 @@ fn zig_delete_tree_min_stack_size_with_kind_hint(
                 let entry = match dir_it.next() {
                     Ok(Some(e)) => e,
                     Ok(None) => break 'dir_it,
+                    // Same as the in-stack walk: the dir vanished mid-iteration.
+                    Err(err) if err.get_errno() == E::ENOENT => break 'dir_it,
                     Err(err) => break 'scan_dir Err(dt_err(err.get_errno())),
                 };
                 let entry_name: Vec<u8> = entry.name.slice().to_vec();

src/sys/dir.rs

@@ -139,15 +139,22 @@ impl Dir {
         });
 
         'process_stack: while let Some(top) = stack.last_mut() {
-            while let Some(entry) = top.iter.next()? {
+            loop {
+                let entry = match top.iter.next() {
+                    Ok(Some(e)) => e,
+                    Ok(None) => break,
+                    // A concurrent deleter removed the directory we iterate:
+                    // getdents on a dead dir reports ENOENT. The rmdir below
+                    // tolerates ENOENT.
+                    Err(e) if e.get_errno() == E::ENOENT => break,
+                    Err(e) => return Err(e),
+                };
                 let mut treat_as_dir = matches!(entry.kind, EntryKind::Directory);
                 'handle_entry: loop {
                     if treat_as_dir {
-                        let new_dir = match openat_a(
+                        let new_dir = match crate::openat_dir_for_delete_tree(
                             top.iter.dir(),
                             entry.name.slice_u8(),
-                            O::DIRECTORY | O::RDONLY | O::CLOEXEC | O::NOFOLLOW,
-                            0,
                         ) {
                             Ok(fd) => fd,
                             Err(e) => match e.get_errno() {
@@ -208,12 +215,7 @@ impl Dir {
             if need_to_retry {
                 // Since we closed the handle that the previous iterator used, we
                 // need to re-open the dir and re-create the iterator.
-                let new_dir = match openat_a(
-                    parent_dir,
-                    &name,
-                    O::DIRECTORY | O::RDONLY | O::CLOEXEC | O::NOFOLLOW,
-                    0,
-                ) {
+                let new_dir = match crate::openat_dir_for_delete_tree(parent_dir, &name) {
                     Ok(fd) => fd,
                     Err(e) => match e.get_errno() {
                         E::ENOTDIR => {
@@ -260,12 +262,7 @@ impl Dir {
                     },
                 }
             } else {
-                return match openat_a(
-                    self.fd,
-                    sub_path,
-                    O::DIRECTORY | O::RDONLY | O::CLOEXEC | O::NOFOLLOW,
-                    0,
-                ) {
+                return match crate::openat_dir_for_delete_tree(self.fd, sub_path) {
                     Ok(fd) => Ok(Some(fd)),
                     Err(e) => match e.get_errno() {
                         E::ENOENT => Ok(None),

src/sys/lib.rs

@@ -6237,6 +6237,9 @@ pub struct WindowsOpenDirOptions {
     pub iterable: bool,
     pub no_follow: bool,
     pub can_rename_or_delete: bool,
+    /// Report `STATUS_DELETE_PENDING`/`STATUS_FILE_DELETED` as ENOENT: a
+    /// directory in that state can never be opened again.
+    pub delete_pending_is_enoent: bool,
     pub op: WindowsOpenDirOp,
 }
 #[cfg(windows)]
@@ -6773,6 +6776,12 @@ pub(crate) fn open_dir_at_windows_nt_path(
             0,
         )
     };
+    if options.delete_pending_is_enoent
+        && (rc == bun_windows_sys::ntstatus::DELETE_PENDING
+            || rc == bun_windows_sys::ntstatus::FILE_DELETED)
+    {
+        return Err(Error::from_code(E::ENOENT, Tag::open));
+    }
     match windows::Win32Error::from_nt_status(rc) {
         windows::Win32Error::SUCCESS => Ok(Fd::from_system(fd)),
         code => Err(Error::from_code(code.to_e(), Tag::open)),
@@ -7018,6 +7027,35 @@ pub fn openat_windows(dir: Fd, path: &[u16], flags: i32, perm: Mode) -> Maybe<Fd
     let norm = normalize_path_windows(dir, path, &mut wbuf.0[..])?;
     openat_windows_impl(dir, norm, flags, perm)
 }
+/// Iterable directory open for the recursive delete-tree walks
+/// (`O_DIRECTORY | O_RDONLY | O_NOFOLLOW` semantics). On Windows
+/// `delete_pending_is_enoent` is set: a dir whose deletion already started
+/// reports ENOENT, like an entry that vanished between readdir and open.
+pub fn openat_dir_for_delete_tree(dir: impl AsFd, path: &[u8]) -> Maybe<Fd> {
+    #[cfg(windows)]
+    {
+        open_dir_at_windows_a(
+            dir,
+            path,
+            WindowsOpenDirOptions {
+                iterable: true,
+                no_follow: true,
+                delete_pending_is_enoent: true,
+                ..Default::default()
+            },
+        )
+    }
+    #[cfg(not(windows))]
+    {
+        openat_a(
+            dir,
+            path,
+            O::DIRECTORY | O::RDONLY | O::CLOEXEC | O::NOFOLLOW,
+            0,
+        )
+    }
+}
+
 /// `openatWindowsA` — UTF-8 input.
 #[cfg(windows)]
 #[inline(never)]

test/js/node/fs/fs.test.ts

@@ -5871,6 +5871,170 @@ const outcome = async fn => {
   expect(exitCode).toBe(0);
 });
 
+// oven-sh/bun#39708, oven-sh/bun#36984: a recursive `rm` that loses a race
+// against a concurrent deleter must keep walking instead of bailing out. The
+// bail-out left the rest of the tree behind (and on Windows surfaced
+// EFAULT/EPERM). A real concurrent deleter is not deterministic, so LD_PRELOAD
+// a shim that plays one through `unlinkat` and `syscall` (the libc symbols on
+// the walk's path; dir opens go through raw syscalls on Linux and cannot be
+// interposed):
+// - a marked file is really unlinked, then reported as ENOENT: the loser of
+//   an unlink race.
+// - a marked dir is really removed, then reported as ENOTEMPTY: rmdir raced a
+//   deleter that was still emptying the dir, and the re-open that follows
+//   finds it gone. The marked dir sits 16 levels deep so the walk reaches it
+//   through the min-stack hand-off, the site that used to abort the walk.
+// - a second marked dir is really removed from inside getdents64, so the
+//   kernel itself reports ENOENT for the directory the walk is iterating
+//   (same mechanism as the readdir shim above).
+// glibc-only, same as the statfs shim above.
+it.skipIf(!isGlibc || !cc)("fs.rm recursive keeps walking when a concurrent deleter wins (#39708)", () => {
+  const deepChain = Array.from({ length: 15 }, (_, i) => `d${i}`).join("/");
+  using dir = tempDir("rm-race-shim", {
+    "shim.c": `
+#define _GNU_SOURCE
+#include <dlfcn.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <limits.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/syscall.h>
+#include <unistd.h>
+
+static int (*next_unlinkat)(int, const char *, int);
+static long (*next_syscall)(long, ...);
+
+int unlinkat(int dirfd, const char *path, int flags) {
+  if (!next_unlinkat) next_unlinkat = dlsym(RTLD_NEXT, "unlinkat");
+  // Interposition probe: report success without deleting anything.
+  if (strstr(path, "bun-shim-probe-keep")) return 0;
+  if (strstr(path, "bun-race-doomed-file")) {
+    // The concurrent deleter wins: the entry is really gone, we get ENOENT.
+    next_unlinkat(dirfd, path, flags);
+    errno = ENOENT;
+    return -1;
+  }
+  if ((flags & AT_REMOVEDIR) && strstr(path, "bun-race-doomed-ne-dir")) {
+    // rmdir raced a deleter that was still emptying the dir: the call
+    // reports ENOTEMPTY, and by the time the caller re-opens the dir to
+    // retry, the deleter has finished and the dir is gone.
+    if (next_unlinkat(dirfd, path, flags) == 0) {
+      errno = ENOTEMPTY;
+      return -1;
+    }
+    return -1;
+  }
+  return next_unlinkat(dirfd, path, flags);
+}
+
+long syscall(long nr, ...) {
+  va_list ap;
+  va_start(ap, nr);
+  long a1 = va_arg(ap, long), a2 = va_arg(ap, long), a3 = va_arg(ap, long);
+  long a4 = va_arg(ap, long), a5 = va_arg(ap, long), a6 = va_arg(ap, long);
+  va_end(ap);
+  if (!next_syscall) next_syscall = dlsym(RTLD_NEXT, "syscall");
+  if (nr == SYS_getdents64) {
+    int fd = (int)a1;
+    char link[64], target[PATH_MAX];
+    snprintf(link, sizeof link, "/proc/self/fd/%d", fd);
+    ssize_t n = readlink(link, target, sizeof target - 1);
+    if (n > 0) {
+      target[n] = 0;
+      const char *base = strrchr(target, '/');
+      base = base ? base + 1 : target;
+      // "probe-getdents" doubles as the interposition probe for this hook:
+      // child.js checks that reading it really removed it.
+      if (strcmp(base, "bun-race-doomed-iter-dir") == 0 ||
+          strcmp(base, "bun-shim-probe-getdents-dir") == 0) {
+        // The concurrent deleter rmdirs the dir mid-iteration; the real
+        // getdents64 on the dead dir then reports ENOENT.
+        rmdir(target);
+      }
+    }
+  }
+  return next_syscall(nr, a1, a2, a3, a4, a5, a6);
+}
+`,
+    "bun-shim-probe-keep.txt": "probe",
+    "root/bun-race-doomed-file.txt": "doomed",
+    "root/keep-a.txt": "x",
+    "root/keep-sub/nested.txt": "x",
+    "child.js": `
+import fs from "node:fs";
+// Probe: the shim swallows unlinkat for this name. If rmSync reports success
+// but the file is still there, the shim interposed the symbol the recursive
+// walk uses. If not, fail loudly instead of passing vacuously.
+fs.rmSync("bun-shim-probe-keep.txt", { recursive: true });
+if (!fs.existsSync("bun-shim-probe-keep.txt")) {
+  console.error("shim did not interpose unlinkat");
+  process.exit(3);
+}
+// Probe for the getdents64 hook: reading this dir makes the shim remove it.
+// The read itself reports the dead-dir ENOENT, which is not the point here.
+try {
+  fs.readdirSync("bun-shim-probe-getdents-dir");
+} catch {}
+if (fs.existsSync("bun-shim-probe-getdents-dir")) {
+  console.error("shim did not interpose syscall(getdents64)");
+  process.exit(3);
+}
+fs.rmSync("root", { recursive: true, force: true });
+if (fs.existsSync("root")) {
+  console.error("leftover tree: " + JSON.stringify(fs.readdirSync("root", { recursive: true })));
+  process.exit(4);
+}
+`,
+  });
+
+  // 15 nested dirs fill the walk's 16-slot stack (root is slot 1), so the
+  // marked dir at the bottom is handed to the min-stack fallback.
+  fs.mkdirSync(path.join(String(dir), "root", deepChain, "bun-race-doomed-ne-dir"), { recursive: true });
+  // Empty dir whose getdents64 the shim turns into the dead-dir ENOENT.
+  fs.mkdirSync(path.join(String(dir), "root", "bun-race-doomed-iter-dir"));
+  fs.mkdirSync(path.join(String(dir), "bun-shim-probe-getdents-dir"));
+
+  const soPath = path.join(String(dir), "shim.so");
+  const compile = Bun.spawnSync({
+    cmd: [cc!, "-shared", "-fPIC", "-o", soPath, path.join(String(dir), "shim.c"), "-ldl"],
+    env: bunEnv,
+  });
+  if (compile.exitCode !== 0) {
+    throw new Error(`Failed to build rm race shim:\n${compile.stderr.toString()}`);
+  }
+
+  const existing = bunEnv.LD_PRELOAD;
+  const proc = Bun.spawnSync({
+    cmd: [bunExe(), "child.js"],
+    env: { ...bunEnv, LD_PRELOAD: existing ? `${soPath}:${existing}` : soPath },
+    cwd: String(dir),
+  });
+  expect(proc.stderr.toString()).toBe("");
+  expect(proc.exitCode).toBe(0);
+});
+
+// The same race without a shim: concurrent rm calls on one tree. Every call
+// must resolve (force: true) and the tree must be gone. On Windows the loser
+// used to reject with EPERM (EFAULT/EACCES before the port) when it opened a
+// directory whose deletion the winner had already started.
+it("concurrent recursive force rm of the same tree all resolve (#39708)", async () => {
+  using dir = tempDir("rm-concurrent", {});
+  for (let round = 0; round < 40; round++) {
+    const target = path.join(String(dir), `victim-${round}`);
+    fs.mkdirSync(path.join(target, "sub"), { recursive: true });
+    fs.writeFileSync(path.join(target, "a.txt"), "x");
+    fs.writeFileSync(path.join(target, "sub", "b.txt"), "x");
+    const results = await Promise.allSettled(
+      Array.from({ length: 8 }, () => fs.promises.rm(target, { recursive: true, force: true })),
+    );
+    const rejected = results.filter(r => r.status === "rejected").map(r => String((r as PromiseRejectedResult).reason));
+    expect(rejected).toEqual([]);
+    expect(fs.existsSync(target)).toBe(false);
+  }
+});
+
 it("fs.Stat constructor", () => {
   expect(new Stats()).toMatchObject({
     "atimeMs": undefined,