Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

fs: make callbacks run in global context

Callbacks that were passed to the binding layer ran in the context of the
(internal) binding object. Make sure they run in the global context.

Before:

  fs.symlink('a', 'b', function() {
    console.log(this); // prints "{ oncomplete: [Function] }"
  });

After:

  fs.symlink('a', 'b', function() {
    console.log(this); // prints "{ <global object> }"
  });
  • Loading branch information...
commit 463d6bac8b349acc462d345a6e298a76f7d06fb1 1 parent c381662
@bnoordhuis bnoordhuis authored
Showing with 53 additions and 38 deletions.
  1. +48 −35 lib/fs.js
  2. +5 −3 test/simple/test-fs-stat.js
View
83 lib/fs.js
@@ -104,7 +104,7 @@ fs.existsSync = function(path) {
fs.readFile = function(path, encoding_) {
var encoding = typeof(encoding_) === 'string' ? encoding_ : null;
var callback = arguments[arguments.length - 1];
- if (typeof(callback) !== 'function') callback = noop;
+ if (typeof(callback) !== 'function') callback = function() {};
// first, stat the file, so we know the size.
var size;
@@ -237,13 +237,27 @@ Object.defineProperty(exports, '_stringToFlags', {
value: stringToFlags
});
-function noop() {}
+
+// Ensure that callbacks run in the global context. Only use this function
+// for callbacks that are passed to the binding layer, callbacks that are
+// invoked from JS already run in the proper scope.
+function makeCallback(cb) {
+ if (typeof cb !== 'function') {
+ // faster than returning a ref to a global no-op function
+ return function() {};
+ }
+
+ return function() {
+ return cb.apply(null, arguments);
+ };
+}
+
// Yes, the follow could be easily DRYed up but I provide the explicit
// list to make the arguments clear.
fs.close = function(fd, callback) {
- binding.close(fd, callback || noop);
+ binding.close(fd, makeCallback(callback));
};
fs.closeSync = function(fd) {
@@ -264,11 +278,7 @@ function modeNum(m, def) {
}
fs.open = function(path, flags, mode, callback) {
- callback = arguments[arguments.length - 1];
- if (typeof(callback) !== 'function') {
- callback = noop;
- }
-
+ callback = makeCallback(arguments[arguments.length - 1]);
mode = modeNum(mode, 438 /*=0666*/);
binding.open(pathModule._makeLong(path),
@@ -376,7 +386,7 @@ fs.writeSync = function(fd, buffer, offset, length, position) {
fs.rename = function(oldPath, newPath, callback) {
binding.rename(pathModule._makeLong(oldPath),
pathModule._makeLong(newPath),
- callback || noop);
+ makeCallback(callback));
};
fs.renameSync = function(oldPath, newPath) {
@@ -385,7 +395,7 @@ fs.renameSync = function(oldPath, newPath) {
};
fs.truncate = function(fd, len, callback) {
- binding.truncate(fd, len, callback || noop);
+ binding.truncate(fd, len, makeCallback(callback));
};
fs.truncateSync = function(fd, len) {
@@ -393,7 +403,7 @@ fs.truncateSync = function(fd, len) {
};
fs.rmdir = function(path, callback) {
- binding.rmdir(pathModule._makeLong(path), callback || noop);
+ binding.rmdir(pathModule._makeLong(path), makeCallback(callback));
};
fs.rmdirSync = function(path) {
@@ -401,7 +411,7 @@ fs.rmdirSync = function(path) {
};
fs.fdatasync = function(fd, callback) {
- binding.fdatasync(fd, callback || noop);
+ binding.fdatasync(fd, makeCallback(callback));
};
fs.fdatasyncSync = function(fd) {
@@ -409,7 +419,7 @@ fs.fdatasyncSync = function(fd) {
};
fs.fsync = function(fd, callback) {
- binding.fsync(fd, callback || noop);
+ binding.fsync(fd, makeCallback(callback));
};
fs.fsyncSync = function(fd) {
@@ -418,8 +428,9 @@ fs.fsyncSync = function(fd) {
fs.mkdir = function(path, mode, callback) {
if (typeof mode === 'function') callback = mode;
- binding.mkdir(pathModule._makeLong(path), modeNum(mode, 511 /*=0777*/),
- callback || noop);
+ binding.mkdir(pathModule._makeLong(path),
+ modeNum(mode, 511 /*=0777*/),
+ makeCallback(callback));
};
fs.mkdirSync = function(path, mode) {
@@ -428,7 +439,7 @@ fs.mkdirSync = function(path, mode) {
};
fs.sendfile = function(outFd, inFd, inOffset, length, callback) {
- binding.sendfile(outFd, inFd, inOffset, length, callback || noop);
+ binding.sendfile(outFd, inFd, inOffset, length, makeCallback(callback));
};
fs.sendfileSync = function(outFd, inFd, inOffset, length) {
@@ -436,7 +447,7 @@ fs.sendfileSync = function(outFd, inFd, inOffset, length) {
};
fs.readdir = function(path, callback) {
- binding.readdir(pathModule._makeLong(path), callback || noop);
+ binding.readdir(pathModule._makeLong(path), makeCallback(callback));
};
fs.readdirSync = function(path) {
@@ -444,15 +455,15 @@ fs.readdirSync = function(path) {
};
fs.fstat = function(fd, callback) {
- binding.fstat(fd, callback || noop);
+ binding.fstat(fd, makeCallback(callback));
};
fs.lstat = function(path, callback) {
- binding.lstat(pathModule._makeLong(path), callback || noop);
+ binding.lstat(pathModule._makeLong(path), makeCallback(callback));
};
fs.stat = function(path, callback) {
- binding.stat(pathModule._makeLong(path), callback || noop);
+ binding.stat(pathModule._makeLong(path), makeCallback(callback));
};
fs.fstatSync = function(fd) {
@@ -468,7 +479,7 @@ fs.statSync = function(path) {
};
fs.readlink = function(path, callback) {
- binding.readlink(pathModule._makeLong(path), callback || noop);
+ binding.readlink(pathModule._makeLong(path), makeCallback(callback));
};
fs.readlinkSync = function(path) {
@@ -477,8 +488,7 @@ fs.readlinkSync = function(path) {
fs.symlink = function(destination, path, type_, callback) {
var type = (typeof(type_) == 'string' ? type_ : null);
- var callback_ = arguments[arguments.length - 1];
- callback = (typeof(callback_) == 'function' ? callback_ : noop);
+ var callback = makeCallback(arguments[arguments.length - 1]);
if (isWindows && type === 'junction') {
destination = pathModule._makeLong(destination);
@@ -500,7 +510,7 @@ fs.symlinkSync = function(destination, path, type) {
fs.link = function(srcpath, dstpath, callback) {
binding.link(pathModule._makeLong(srcpath),
pathModule._makeLong(dstpath),
- callback || noop);
+ makeCallback(callback));
};
fs.linkSync = function(srcpath, dstpath) {
@@ -509,7 +519,7 @@ fs.linkSync = function(srcpath, dstpath) {
};
fs.unlink = function(path, callback) {
- binding.unlink(pathModule._makeLong(path), callback || noop);
+ binding.unlink(pathModule._makeLong(path), makeCallback(callback));
};
fs.unlinkSync = function(path) {
@@ -517,7 +527,7 @@ fs.unlinkSync = function(path) {
};
fs.fchmod = function(fd, mode, callback) {
- binding.fchmod(fd, modeNum(mode), callback || noop);
+ binding.fchmod(fd, modeNum(mode), makeCallback(callback));
};
fs.fchmodSync = function(fd, mode) {
@@ -526,7 +536,7 @@ fs.fchmodSync = function(fd, mode) {
if (constants.hasOwnProperty('O_SYMLINK')) {
fs.lchmod = function(path, mode, callback) {
- callback = callback || noop;
+ callback = callback || (function() {});
fs.open(path, constants.O_WRONLY | constants.O_SYMLINK, function(err, fd) {
if (err) {
callback(err);
@@ -565,7 +575,9 @@ if (constants.hasOwnProperty('O_SYMLINK')) {
fs.chmod = function(path, mode, callback) {
- binding.chmod(pathModule._makeLong(path), modeNum(mode), callback || noop);
+ binding.chmod(pathModule._makeLong(path),
+ modeNum(mode),
+ makeCallback(callback));
};
fs.chmodSync = function(path, mode) {
@@ -574,7 +586,7 @@ fs.chmodSync = function(path, mode) {
if (constants.hasOwnProperty('O_SYMLINK')) {
fs.lchown = function(path, uid, gid, callback) {
- callback = callback || noop;
+ callback = callback || (function() {});
fs.open(path, constants.O_WRONLY | constants.O_SYMLINK, function(err, fd) {
if (err) {
callback(err);
@@ -591,7 +603,7 @@ if (constants.hasOwnProperty('O_SYMLINK')) {
}
fs.fchown = function(fd, uid, gid, callback) {
- binding.fchown(fd, uid, gid, callback || noop);
+ binding.fchown(fd, uid, gid, makeCallback(callback));
};
fs.fchownSync = function(fd, uid, gid) {
@@ -599,7 +611,7 @@ fs.fchownSync = function(fd, uid, gid) {
};
fs.chown = function(path, uid, gid, callback) {
- binding.chown(pathModule._makeLong(path), uid, gid, callback || noop);
+ binding.chown(pathModule._makeLong(path), uid, gid, makeCallback(callback));
};
fs.chownSync = function(path, uid, gid) {
@@ -622,9 +634,10 @@ function toUnixTimestamp(time) {
fs._toUnixTimestamp = toUnixTimestamp;
fs.utimes = function(path, atime, mtime, callback) {
- atime = toUnixTimestamp(atime);
- mtime = toUnixTimestamp(mtime);
- binding.utimes(pathModule._makeLong(path), atime, mtime, callback || noop);
+ binding.utimes(pathModule._makeLong(path),
+ toUnixTimestamp(atime),
+ toUnixTimestamp(mtime),
+ makeCallback(callback));
};
fs.utimesSync = function(path, atime, mtime) {
@@ -636,7 +649,7 @@ fs.utimesSync = function(path, atime, mtime) {
fs.futimes = function(fd, atime, mtime, callback) {
atime = toUnixTimestamp(atime);
mtime = toUnixTimestamp(mtime);
- binding.futimes(fd, atime, mtime, callback || noop);
+ binding.futimes(fd, atime, mtime, makeCallback(callback));
};
fs.futimesSync = function(fd, atime, mtime) {
View
8 test/simple/test-fs-stat.js
@@ -19,9 +19,6 @@
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
// USE OR OTHER DEALINGS IN THE SOFTWARE.
-
-
-
var common = require('../common');
var assert = require('assert');
var fs = require('fs');
@@ -36,6 +33,7 @@ fs.stat('.', function(err, stats) {
assert.ok(stats.mtime instanceof Date);
success_count++;
}
+ assert(this === global);
});
fs.lstat('.', function(err, stats) {
@@ -46,6 +44,7 @@ fs.lstat('.', function(err, stats) {
assert.ok(stats.mtime instanceof Date);
success_count++;
}
+ assert(this === global);
});
// fstat
@@ -62,7 +61,10 @@ fs.open('.', 'r', undefined, function(err, fd) {
success_count++;
fs.close(fd);
}
+ assert(this === global);
});
+
+ assert(this === global);
});
// fstatSync
Please sign in to comment.
Something went wrong with that request. Please try again.