Skip to content

fs: rmSync with recursive returns without throwing while the directory still exists #67010

Open

Description

@arieller

Version

v24.22.1 and v26.12.1 (also v23.11.1 and v24.0.0). Not v22.23.4.

Platform

Linux 6.18.33.2-microsoft-standard-WSL2 x86_64

Subsystem

fs

What steps will reproduce the bug?

Call fs.rmSync(dir, { recursive: true, force: true }) while another thread keeps creating and removing one file inside dir.

The script below does that. A worker churns a file under dir/busy/, the main thread calls rmSync on dir, and then checks whether dir still exists once the call has returned.

node repro.mjs 300
repro.mjs
// fs.rmSync(dir, { recursive: true, force: true }) returns without throwing
// while `dir` still exists, when another thread removes an entry inside `dir`
// during the walk.
import { existsSync, mkdirSync, mkdtempSync, readdirSync, rmSync, unlinkSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { Worker, isMainThread, workerData } from "node:worker_threads";

if (!isMainThread) {
  // Churn: keep creating and removing one file inside the tree being removed.
  const { lock, stop } = workerData;
  const flag = new Int32Array(stop);
  while (Atomics.load(flag, 0) === 0) {
    try {
      writeFileSync(lock, "");
      unlinkSync(lock);
    } catch {}
  }
  process.exit(0);
}

const TRIALS = Number(process.argv[2] ?? 200);
const counts = { removed: 0, threw: 0, survivedSilently: 0 };
let example;

for (let i = 0; i < TRIALS; i++) {
  const dir = mkdtempSync(join(tmpdir(), "rmsync-"));
  const busy = join(dir, "busy");
  mkdirSync(busy);
  for (let d = 0; d < 20; d++) {
    mkdirSync(join(dir, `d${d}`));
    for (let f = 0; f < 20; f++) writeFileSync(join(dir, `d${d}`, `f${f}`), "x");
  }
  const stop = new SharedArrayBuffer(4);
  const worker = new Worker(new URL(import.meta.url), { workerData: { lock: join(busy, "lock"), stop } });
  await new Promise((resolve) => worker.once("online", resolve));

  try {
    rmSync(dir, { recursive: true, force: true });
    if (existsSync(dir)) {
      counts.survivedSilently++;
      example ??= readdirSync(dir, { recursive: true });
    } else {
      counts.removed++;
    }
  } catch (error) {
    counts.threw++;
  }

  Atomics.store(new Int32Array(stop), 0, 1);
  await new Promise((resolve) => worker.once("exit", resolve));
  rmSync(dir, { recursive: true, force: true, maxRetries: 5 });
}

console.log(`node ${process.version}`, JSON.stringify(counts));
if (example) console.log("left behind after rmSync returned:", JSON.stringify(example));

How often does it reproduce? Is there a required condition?

Roughly one call in five on this machine. Out of 300 calls, 65 on v24.22.1 and 61 on v26.12.1 returned with the directory still there. On v22.23.4 it was 0.

It needs another process or thread removing an entry inside the tree while rmSync walks it. Any program that removes a directory while something still writes into it can hit this. I found it through git commit. Since git 2.47 it leaves a detached git maintenance process touching .git/objects after the command returns, and a test that removed its fixture repo right after committing sometimes kept the repo.

What is the expected behavior? Why is that the expected behavior?

Either dir is gone when rmSync returns, or rmSync throws.

The docs for force say a missing path is ignored, and maxRetries lists the errors that are retried. Nothing allows a successful return with the path still there. v22 removed the rest of the tree in this situation. Its JS implementation skipped an entry that had vanished and carried on.

What do you see instead?

rmSync returns normally and dir is still on disk, partly removed:

node v24.22.1 {"removed":196,"threw":39,"survivedSilently":65}
left behind after rmSync returned: ["busy","d1","d10",...,"busy/lock","d1/f0",...]

Additional information

RmSync in src/node_file.cc calls std::filesystem::remove_all and treats no_such_file_or_directory as success:

if (!error || error == std::errc::no_such_file_or_directory) {
  return;
}

remove_all reports that error for an entry that disappeared during the walk as well as for a missing target, and it stops at that entry. Only the missing target means the path is gone. This came in with the C++ implementation in #53617, so v23 onward.

Proposed fix here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions