Skip to content

Commit 7168295

Browse files
anonriganonrig
authoredJul 18, 2024··
fs: move rmSync implementation to c++
PR-URL: #53617 Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
·
v23.11.0v23.0.0
1 parent cafd44d commit 7168295

File tree

6 files changed

+114
-177
lines changed

6 files changed

+114
-177
lines changed
 

Diff for: ‎lib/fs.js

+4-9
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,6 @@ let promises = null;
161161
let ReadStream;
162162
let WriteStream;
163163
let rimraf;
164-
let rimrafSync;
165164
let kResistStopPropagation;
166165

167166
// These have to be separate because of how graceful-fs happens to do it's
@@ -1124,7 +1123,7 @@ function lazyLoadCp() {
11241123

11251124
function lazyLoadRimraf() {
11261125
if (rimraf === undefined)
1127-
({ rimraf, rimrafSync } = require('internal/fs/rimraf'));
1126+
({ rimraf } = require('internal/fs/rimraf'));
11281127
}
11291128

11301129
/**
@@ -1192,8 +1191,7 @@ function rmdirSync(path, options) {
11921191
emitRecursiveRmdirWarning();
11931192
options = validateRmOptionsSync(path, { ...options, force: false }, true);
11941193
if (options !== false) {
1195-
lazyLoadRimraf();
1196-
return rimrafSync(path, options);
1194+
return binding.rmSync(path, options.maxRetries, options.recursive, options.retryDelay);
11971195
}
11981196
} else {
11991197
validateRmdirOptions(options);
@@ -1244,11 +1242,8 @@ function rm(path, options, callback) {
12441242
* @returns {void}
12451243
*/
12461244
function rmSync(path, options) {
1247-
lazyLoadRimraf();
1248-
return rimrafSync(
1249-
getValidatedPath(path),
1250-
validateRmOptionsSync(path, options, false),
1251-
);
1245+
const opts = validateRmOptionsSync(path, options, false);
1246+
return binding.rmSync(getValidatedPath(path), opts.maxRetries, opts.recursive, opts.retryDelay);
12521247
}
12531248

12541249
/**

Diff for: ‎lib/internal/fs/rimraf.js

+2-143
Original file line numberDiff line numberDiff line change
@@ -16,26 +16,19 @@ const { Buffer } = require('buffer');
1616
const fs = require('fs');
1717
const {
1818
chmod,
19-
chmodSync,
2019
lstat,
21-
lstatSync,
2220
readdir,
23-
readdirSync,
2421
rmdir,
25-
rmdirSync,
2622
stat,
27-
statSync,
2823
unlink,
29-
unlinkSync,
3024
} = fs;
3125
const { sep } = require('path');
3226
const { setTimeout } = require('timers');
33-
const { sleep, isWindows } = require('internal/util');
27+
const { isWindows } = require('internal/util');
3428
const notEmptyErrorCodes = new SafeSet(['ENOTEMPTY', 'EEXIST', 'EPERM']);
3529
const retryErrorCodes = new SafeSet(
3630
['EBUSY', 'EMFILE', 'ENFILE', 'ENOTEMPTY', 'EPERM']);
3731
const epermHandler = isWindows ? fixWinEPERM : _rmdir;
38-
const epermHandlerSync = isWindows ? fixWinEPERMSync : _rmdirSync;
3932
const readdirEncoding = 'buffer';
4033
const separator = Buffer.from(sep);
4134

@@ -172,138 +165,4 @@ function rimrafPromises(path, options) {
172165
}
173166

174167

175-
function rimrafSync(path, options) {
176-
let stats;
177-
178-
try {
179-
stats = lstatSync(path);
180-
} catch (err) {
181-
if (err.code === 'ENOENT')
182-
return;
183-
184-
// Windows can EPERM on stat.
185-
if (isWindows && err.code === 'EPERM')
186-
fixWinEPERMSync(path, options, err);
187-
}
188-
189-
try {
190-
// SunOS lets the root user unlink directories.
191-
if (stats?.isDirectory())
192-
_rmdirSync(path, options, null);
193-
else
194-
_unlinkSync(path, options);
195-
} catch (err) {
196-
if (err.code === 'ENOENT')
197-
return;
198-
if (err.code === 'EPERM')
199-
return epermHandlerSync(path, options, err);
200-
if (err.code !== 'EISDIR')
201-
throw err;
202-
203-
_rmdirSync(path, options, err);
204-
}
205-
}
206-
207-
208-
function _unlinkSync(path, options) {
209-
const tries = options.maxRetries + 1;
210-
211-
for (let i = 1; i <= tries; i++) {
212-
try {
213-
return unlinkSync(path);
214-
} catch (err) {
215-
// Only sleep if this is not the last try, and the delay is greater
216-
// than zero, and an error was encountered that warrants a retry.
217-
if (retryErrorCodes.has(err.code) &&
218-
i < tries &&
219-
options.retryDelay > 0) {
220-
sleep(i * options.retryDelay);
221-
} else if (err.code === 'ENOENT') {
222-
// The file is already gone.
223-
return;
224-
} else if (i === tries) {
225-
throw err;
226-
}
227-
}
228-
}
229-
}
230-
231-
232-
function _rmdirSync(path, options, originalErr) {
233-
try {
234-
rmdirSync(path);
235-
} catch (err) {
236-
if (err.code === 'ENOENT')
237-
return;
238-
if (err.code === 'ENOTDIR') {
239-
throw originalErr || err;
240-
}
241-
242-
if (notEmptyErrorCodes.has(err.code)) {
243-
// Removing failed. Try removing all children and then retrying the
244-
// original removal. Windows has a habit of not closing handles promptly
245-
// when files are deleted, resulting in spurious ENOTEMPTY failures. Work
246-
// around that issue by retrying on Windows.
247-
const pathBuf = Buffer.from(path);
248-
249-
ArrayPrototypeForEach(readdirSync(pathBuf, readdirEncoding), (child) => {
250-
const childPath = Buffer.concat([pathBuf, separator, child]);
251-
252-
rimrafSync(childPath, options);
253-
});
254-
255-
const tries = options.maxRetries + 1;
256-
257-
for (let i = 1; i <= tries; i++) {
258-
try {
259-
return fs.rmdirSync(path);
260-
} catch (err) {
261-
// Only sleep if this is not the last try, and the delay is greater
262-
// than zero, and an error was encountered that warrants a retry.
263-
if (retryErrorCodes.has(err.code) &&
264-
i < tries &&
265-
options.retryDelay > 0) {
266-
sleep(i * options.retryDelay);
267-
} else if (err.code === 'ENOENT') {
268-
// The file is already gone.
269-
return;
270-
} else if (i === tries) {
271-
throw err;
272-
}
273-
}
274-
}
275-
}
276-
277-
throw originalErr || err;
278-
}
279-
}
280-
281-
282-
function fixWinEPERMSync(path, options, originalErr) {
283-
try {
284-
chmodSync(path, 0o666);
285-
} catch (err) {
286-
if (err.code === 'ENOENT')
287-
return;
288-
289-
throw originalErr;
290-
}
291-
292-
let stats;
293-
294-
try {
295-
stats = statSync(path, { throwIfNoEntry: false });
296-
} catch {
297-
throw originalErr;
298-
}
299-
300-
if (stats === undefined) return;
301-
302-
if (stats.isDirectory())
303-
_rmdirSync(path, options, originalErr);
304-
else
305-
_unlinkSync(path, options);
306-
}
307-
308-
309-
module.exports = { rimraf, rimrafPromises, rimrafSync };
168+
module.exports = { rimraf, rimrafPromises };

Diff for: ‎src/node_errors.h

+1
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ void OOMErrorHandler(const char* location, const v8::OOMDetails& details);
7070
V(ERR_DLOPEN_FAILED, Error) \
7171
V(ERR_ENCODING_INVALID_ENCODED_DATA, TypeError) \
7272
V(ERR_EXECUTION_ENVIRONMENT_NOT_AVAILABLE, Error) \
73+
V(ERR_FS_EISDIR, Error) \
7374
V(ERR_ILLEGAL_CONSTRUCTOR, Error) \
7475
V(ERR_INVALID_ADDRESS, Error) \
7576
V(ERR_INVALID_ARG_VALUE, TypeError) \

Diff for: ‎src/node_file.cc

+104
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,12 @@
4848
# include <io.h>
4949
#endif
5050

51+
#ifdef _WIN32
52+
#include <windows.h>
53+
#else
54+
#include <unistd.h>
55+
#endif
56+
5157
namespace node {
5258

5359
namespace fs {
@@ -1624,6 +1630,102 @@ static void RMDir(const FunctionCallbackInfo<Value>& args) {
16241630
}
16251631
}
16261632

1633+
static void RmSync(const FunctionCallbackInfo<Value>& args) {
1634+
Environment* env = Environment::GetCurrent(args);
1635+
Isolate* isolate = env->isolate();
1636+
1637+
CHECK_EQ(args.Length(), 4); // path, maxRetries, recursive, retryDelay
1638+
1639+
BufferValue path(isolate, args[0]);
1640+
CHECK_NOT_NULL(*path);
1641+
ToNamespacedPath(env, &path);
1642+
THROW_IF_INSUFFICIENT_PERMISSIONS(
1643+
env, permission::PermissionScope::kFileSystemWrite, path.ToStringView());
1644+
auto file_path = std::filesystem::path(path.ToStringView());
1645+
std::error_code error;
1646+
auto file_status = std::filesystem::status(file_path, error);
1647+
1648+
if (file_status.type() == std::filesystem::file_type::not_found) {
1649+
return;
1650+
}
1651+
1652+
int maxRetries = args[1].As<Int32>()->Value();
1653+
int recursive = args[2]->IsTrue();
1654+
int retryDelay = args[3].As<Int32>()->Value();
1655+
1656+
// File is a directory and recursive is false
1657+
if (file_status.type() == std::filesystem::file_type::directory &&
1658+
!recursive) {
1659+
return THROW_ERR_FS_EISDIR(
1660+
isolate, "Path is a directory: %s", file_path.c_str());
1661+
}
1662+
1663+
// Allowed errors are:
1664+
// - EBUSY: std::errc::device_or_resource_busy
1665+
// - EMFILE: std::errc::too_many_files_open
1666+
// - ENFILE: std::errc::too_many_files_open_in_system
1667+
// - ENOTEMPTY: std::errc::directory_not_empty
1668+
// - EPERM: std::errc::operation_not_permitted
1669+
auto can_omit_error = [](std::error_code error) -> bool {
1670+
return (error == std::errc::device_or_resource_busy ||
1671+
error == std::errc::too_many_files_open ||
1672+
error == std::errc::too_many_files_open_in_system ||
1673+
error == std::errc::directory_not_empty ||
1674+
error == std::errc::operation_not_permitted);
1675+
};
1676+
1677+
while (maxRetries >= 0) {
1678+
if (recursive) {
1679+
std::filesystem::remove_all(file_path, error);
1680+
} else {
1681+
std::filesystem::remove(file_path, error);
1682+
}
1683+
1684+
if (!error || error == std::errc::no_such_file_or_directory) {
1685+
return;
1686+
} else if (!can_omit_error(error)) {
1687+
break;
1688+
}
1689+
1690+
if (retryDelay != 0) {
1691+
#ifdef _WIN32
1692+
Sleep(retryDelay / 1000);
1693+
#else
1694+
sleep(retryDelay / 1000);
1695+
#endif
1696+
}
1697+
maxRetries--;
1698+
}
1699+
1700+
// This is required since std::filesystem::path::c_str() returns different
1701+
// values in Windows and Unix.
1702+
#ifdef _WIN32
1703+
auto file_ = file_path.string().c_str();
1704+
int permission_denied_error = EPERM;
1705+
#else
1706+
auto file_ = file_path.c_str();
1707+
int permission_denied_error = EACCES;
1708+
#endif // !_WIN32
1709+
1710+
if (error == std::errc::operation_not_permitted) {
1711+
std::string message = "Operation not permitted: " + file_path.string();
1712+
return env->ThrowErrnoException(EPERM, "rm", message.c_str(), file_);
1713+
} else if (error == std::errc::directory_not_empty) {
1714+
std::string message = "Directory not empty: " + file_path.string();
1715+
return env->ThrowErrnoException(EACCES, "rm", message.c_str(), file_);
1716+
} else if (error == std::errc::not_a_directory) {
1717+
std::string message = "Not a directory: " + file_path.string();
1718+
return env->ThrowErrnoException(ENOTDIR, "rm", message.c_str(), file_);
1719+
} else if (error == std::errc::permission_denied) {
1720+
std::string message = "Permission denied: " + file_path.string();
1721+
return env->ThrowErrnoException(
1722+
permission_denied_error, "rm", message.c_str(), file_);
1723+
}
1724+
1725+
std::string message = "Unknown error: " + error.message();
1726+
return env->ThrowErrnoException(UV_UNKNOWN, "rm", message.c_str(), file_);
1727+
}
1728+
16271729
int MKDirpSync(uv_loop_t* loop,
16281730
uv_fs_t* req,
16291731
const std::string& path,
@@ -3342,6 +3444,7 @@ static void CreatePerIsolateProperties(IsolateData* isolate_data,
33423444
SetMethod(isolate, target, "rename", Rename);
33433445
SetMethod(isolate, target, "ftruncate", FTruncate);
33443446
SetMethod(isolate, target, "rmdir", RMDir);
3447+
SetMethod(isolate, target, "rmSync", RmSync);
33453448
SetMethod(isolate, target, "mkdir", MKDir);
33463449
SetMethod(isolate, target, "readdir", ReadDir);
33473450
SetFastMethod(isolate,
@@ -3468,6 +3571,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
34683571
registry->Register(Rename);
34693572
registry->Register(FTruncate);
34703573
registry->Register(RMDir);
3574+
registry->Register(RmSync);
34713575
registry->Register(MKDir);
34723576
registry->Register(ReadDir);
34733577
registry->Register(InternalModuleStat);

Diff for: ‎test/parallel/test-fs-rmdir-recursive.js

-25
Original file line numberDiff line numberDiff line change
@@ -217,28 +217,3 @@ function removeAsync(dir) {
217217
message: /^The value of "options\.maxRetries" is out of range\./
218218
});
219219
}
220-
221-
// It should not pass recursive option to rmdirSync, when called from
222-
// rimraf (see: #35566)
223-
{
224-
// Make a non-empty directory:
225-
const original = fs.rmdirSync;
226-
const dir = `${nextDirPath()}/foo/bar`;
227-
fs.mkdirSync(dir, { recursive: true });
228-
fs.writeFileSync(`${dir}/foo.txt`, 'hello world', 'utf8');
229-
230-
// When called the second time from rimraf, the recursive option should
231-
// not be set for rmdirSync:
232-
let callCount = 0;
233-
let rmdirSyncOptionsFromRimraf;
234-
fs.rmdirSync = (path, options) => {
235-
if (callCount > 0) {
236-
rmdirSyncOptionsFromRimraf = { ...options };
237-
}
238-
callCount++;
239-
return original(path, options);
240-
};
241-
fs.rmdirSync(dir, { recursive: true });
242-
fs.rmdirSync = original;
243-
assert.strictEqual(rmdirSyncOptionsFromRimraf.recursive, undefined);
244-
}

Diff for: ‎typings/internalBinding/fs.d.ts

+3
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,8 @@ declare namespace InternalFSBinding {
196196
function rmdir(path: string): void;
197197
function rmdir(path: string, usePromises: typeof kUsePromises): Promise<void>;
198198

199+
function rmSync(path: StringOrBuffer, maxRetries: number, recursive: boolean, retryDelay: number): void;
200+
199201
function stat(path: StringOrBuffer, useBigint: boolean, req: FSReqCallback<Float64Array | BigUint64Array>): void;
200202
function stat(path: StringOrBuffer, useBigint: true, req: FSReqCallback<BigUint64Array>): void;
201203
function stat(path: StringOrBuffer, useBigint: false, req: FSReqCallback<Float64Array>): void;
@@ -279,6 +281,7 @@ export interface FsBinding {
279281
realpath: typeof InternalFSBinding.realpath;
280282
rename: typeof InternalFSBinding.rename;
281283
rmdir: typeof InternalFSBinding.rmdir;
284+
rmSync: typeof InternalFSBinding.rmSync;
282285
stat: typeof InternalFSBinding.stat;
283286
symlink: typeof InternalFSBinding.symlink;
284287
unlink: typeof InternalFSBinding.unlink;

0 commit comments

Comments
 (0)
Please sign in to comment.