Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fs: do not throw exception after creating FSReqCallback #35487

Closed
wants to merge 2 commits into from
Closed

fs: do not throw exception after creating FSReqCallback #35487

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8a58544
fs: do not throw exception after creating FSReqCallback
addaleax Oct 3, 2020
4f5c989
fixup! fs: do not throw exception after creating FSReqCallback
addaleax Oct 4, 2020
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
75 changes: 48 additions & 27 deletions lib/fs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@
// See https://github.com/libuv/libuv/pull/1501.
const kIoMaxLength = 2 ** 31 - 1;

// When using FSReqCallback, make sure to create the object only *after* all
// parameter validation has happened, so that the objects are not kept in memory
// in case they are created but never used due to an exception.

const {
Map,
MathMax,
Expand Down Expand Up @@ -198,8 +202,10 @@ function access(path, mode, callback) {

path = getValidatedPath(path);
mode = getValidMode(mode, 'access');
callback = makeCallback(callback);

const req = new FSReqCallback();
req.oncomplete = makeCallback(callback);
req.oncomplete = callback;
binding.access(pathModule.toNamespacedPath(path), mode, req);
}

Expand Down Expand Up @@ -307,19 +313,19 @@ function readFile(path, options, callback) {
const context = new ReadFileContext(callback, options.encoding);
context.isUserFd = isFd(path); // File descriptor ownership

const req = new FSReqCallback();
req.context = context;
req.oncomplete = readFileAfterOpen;

if (context.isUserFd) {
process.nextTick(function tick() {
req.oncomplete(null, path);
});
process.nextTick(function tick(context) {
readFileAfterOpen.call({ context }, null, path);
}, context);
return;
}

path = getValidatedPath(path);
const flagsNumber = stringToFlags(options.flags);
path = getValidatedPath(path);

const req = new FSReqCallback();
req.context = context;
req.oncomplete = readFileAfterOpen;
binding.open(pathModule.toNamespacedPath(path),
flagsNumber,
0o666,
Expand Down Expand Up @@ -416,8 +422,10 @@ function readFileSync(path, options) {

function close(fd, callback) {
validateInt32(fd, 'fd', 0);
callback = makeCallback(callback);

const req = new FSReqCallback();
req.oncomplete = makeCallback(callback);
req.oncomplete = callback;
binding.close(fd, req);
}

Expand Down Expand Up @@ -590,12 +598,11 @@ function readv(fd, buffers, position, callback) {

validateInt32(fd, 'fd', /* min */ 0);
validateBufferArray(buffers);
callback = maybeCallback(callback || position);

const req = new FSReqCallback();
req.oncomplete = wrapper;

callback = maybeCallback(callback || position);

if (typeof position !== 'number')
position = null;

Expand Down Expand Up @@ -712,12 +719,11 @@ function writev(fd, buffers, position, callback) {

validateInt32(fd, 'fd', 0);
validateBufferArray(buffers);
callback = maybeCallback(callback || position);

const req = new FSReqCallback();
req.oncomplete = wrapper;

callback = maybeCallback(callback || position);

if (typeof position !== 'number')
position = null;

Expand Down Expand Up @@ -819,8 +825,10 @@ function ftruncate(fd, len = 0, callback) {
validateInt32(fd, 'fd', 0);
validateInteger(len, 'len');
len = MathMax(0, len);
callback = makeCallback(callback);

const req = new FSReqCallback();
req.oncomplete = makeCallback(callback);
req.oncomplete = callback;
binding.ftruncate(fd, len, req);
}

Expand Down Expand Up @@ -989,8 +997,10 @@ function fstat(fd, options = { bigint: false }, callback) {
options = {};
}
validateInt32(fd, 'fd', 0);
callback = makeStatsCallback(callback);

const req = new FSReqCallback(options.bigint);
req.oncomplete = makeStatsCallback(callback);
req.oncomplete = callback;
binding.fstat(fd, options.bigint, req);
}

Expand All @@ -1001,6 +1011,7 @@ function lstat(path, options = { bigint: false }, callback) {
}
callback = makeStatsCallback(callback);
path = getValidatedPath(path);

const req = new FSReqCallback(options.bigint);
req.oncomplete = callback;
binding.lstat(pathModule.toNamespacedPath(path), options.bigint, req);
Expand All @@ -1013,6 +1024,7 @@ function stat(path, options = { bigint: false }, callback) {
}
callback = makeStatsCallback(callback);
path = getValidatedPath(path);

const req = new FSReqCallback(options.bigint);
req.oncomplete = callback;
binding.stat(pathModule.toNamespacedPath(path), options.bigint, req);
Expand Down Expand Up @@ -1070,9 +1082,6 @@ function symlink(target, path, type_, callback_) {
target = getValidatedPath(target, 'target');
path = getValidatedPath(path);

const req = new FSReqCallback();
req.oncomplete = callback;

if (isWindows && type === null) {
let absoluteTarget;
try {
Expand All @@ -1087,18 +1096,25 @@ function symlink(target, path, type_, callback_) {
stat(absoluteTarget, (err, stat) => {
const resolvedType = !err && stat.isDirectory() ? 'dir' : 'file';
const resolvedFlags = stringToSymlinkType(resolvedType);
binding.symlink(preprocessSymlinkDestination(target,
resolvedType,
path),
const destination = preprocessSymlinkDestination(target,
resolvedType,
path);

const req = new FSReqCallback();
req.oncomplete = callback;
binding.symlink(destination,
pathModule.toNamespacedPath(path), resolvedFlags, req);
});
return;
}
}

const destination = preprocessSymlinkDestination(target, type, path);

const flags = stringToSymlinkType(type);
binding.symlink(preprocessSymlinkDestination(target, type, path),
pathModule.toNamespacedPath(path), flags, req);
const req = new FSReqCallback();
req.oncomplete = callback;
binding.symlink(destination, pathModule.toNamespacedPath(path), flags, req);
}

function symlinkSync(target, path, type) {
Expand Down Expand Up @@ -1255,9 +1271,10 @@ function fchown(fd, uid, gid, callback) {
validateInt32(fd, 'fd', 0);
validateInteger(uid, 'uid', -1, kMaxUserId);
validateInteger(gid, 'gid', -1, kMaxUserId);
callback = makeCallback(callback);

const req = new FSReqCallback();
req.oncomplete = makeCallback(callback);
req.oncomplete = callback;
binding.fchown(fd, uid, gid, req);
}

Expand Down Expand Up @@ -1316,8 +1333,10 @@ function futimes(fd, atime, mtime, callback) {
validateInt32(fd, 'fd', 0);
atime = toUnixTimestamp(atime, 'atime');
mtime = toUnixTimestamp(mtime, 'mtime');
callback = makeCallback(callback);

const req = new FSReqCallback();
req.oncomplete = makeCallback(callback);
req.oncomplete = callback;
binding.futimes(fd, atime, mtime, req);
}

Expand Down Expand Up @@ -1923,8 +1942,10 @@ function copyFile(src, dest, mode, callback) {
src = pathModule._makeLong(src);
dest = pathModule._makeLong(dest);
mode = getValidMode(mode, 'copyFile');
callback = makeCallback(callback);

const req = new FSReqCallback();
req.oncomplete = makeCallback(callback);
req.oncomplete = callback;
binding.copyFile(src, dest, mode, req);
}

Expand Down
14 changes: 7 additions & 7 deletions lib/internal/fs/read_file_context.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,18 +100,18 @@ class ReadFileContext {
}

close(err) {
if (this.isUserFd) {
process.nextTick(function tick(context) {
readFileAfterClose.call({ context }, null);
}, this);
return;
}

const req = new FSReqCallback();
req.oncomplete = readFileAfterClose;
req.context = this;
this.err = err;

if (this.isUserFd) {
process.nextTick(function tick() {
req.oncomplete(null);
});
return;
}

close(this.fd, req);
}
}
Expand Down