diff --git a/src/node_file.cc b/src/node_file.cc index f5e7cc80e72..8d59e027f90 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -1884,6 +1884,7 @@ static void RmSync(const FunctionCallbackInfo& args) { }; int i = 1; + bool rewalked = false; #ifdef _WIN32 bool cleared_readonly = false; #endif @@ -1911,9 +1912,22 @@ static void RmSync(const FunctionCallbackInfo& args) { } #endif // _WIN32 - if (!error || error == std::errc::no_such_file_or_directory) { + if (!error) { return; - } else if (!can_omit_error(error)) { + } + if (error == std::errc::no_such_file_or_directory) { + std::error_code probe; + if (std::filesystem::symlink_status(file_path, probe).type() == + std::filesystem::file_type::not_found) { + return; + } + if (!rewalked) { + rewalked = true; + continue; + } + error = std::make_error_code(std::errc::directory_not_empty); + } + if (!can_omit_error(error)) { break; } diff --git a/test/parallel/test-fs-rm.js b/test/parallel/test-fs-rm.js index 80600d114b9..11df1e6dc25 100644 --- a/test/parallel/test-fs-rm.js +++ b/test/parallel/test-fs-rm.js @@ -657,3 +657,72 @@ if (isGitPresent) { // Clean up parent directory fs.rmSync(dirname, { recursive: true, force: true }); } + +// An entry that another thread removes during the walk must not end the +// removal early: when rmSync returns, the directory is gone. +{ + const { Worker } = require('worker_threads'); + const retried = ['EBUSY', 'EMFILE', 'ENFILE', 'ENOTEMPTY', 'EPERM']; + const CHURN = 0; + const PAUSE = 1; + const STOP = 2; + const command = new Int32Array(new SharedArrayBuffer(4)); + const parked = new Int32Array(new SharedArrayBuffer(4)); + const dir = nextDirPath('rm-churn'); + const lock = path.join(dir, 'busy', 'lock'); + const worker = new Worker(` + const { workerData } = require('worker_threads'); + const fs = require('fs'); + const command = new Int32Array(workerData.command); + const parked = new Int32Array(workerData.parked); + for (;;) { + const now = Atomics.load(command, 0); + if (now === ${STOP}) break; + if (now === ${PAUSE}) { + Atomics.store(parked, 0, 1); + Atomics.notify(parked, 0); + Atomics.wait(command, 0, ${PAUSE}); + Atomics.store(parked, 0, 0); + continue; + } + try { + fs.writeFileSync(workerData.lock, ''); + fs.unlinkSync(workerData.lock); + } catch { + // The directory may be gone already. + } + } + `, { eval: true, workerData: { command: command.buffer, parked: parked.buffer, lock } }); + const pause = () => { + Atomics.store(command, 0, PAUSE); + Atomics.wait(parked, 0, 0); + }; + const churn = () => { + Atomics.store(command, 0, CHURN); + Atomics.notify(command, 0); + }; + worker.once('online', common.mustCall(() => { + try { + for (let round = 0; round < 50; round++) { + makeNonEmptyDirectory(2, 10, 2, dir, false); + fs.mkdirSync(path.join(dir, 'busy')); + churn(); + let thrown; + try { + fs.rmSync(dir, { recursive: true, force: true, maxRetries: 10, retryDelay: 10 }); + } catch (err) { + thrown = err; + } + pause(); + if (thrown) { + assert(retried.includes(thrown.code), `rmSync threw ${thrown.code}`); + fs.rmSync(dir, { recursive: true, force: true }); + } + assert.strictEqual(fs.existsSync(dir), false); + } + } finally { + Atomics.store(command, 0, STOP); + Atomics.notify(command, 0); + } + })); +}