Permalink
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...
1 parent c381662 commit 463d6bac8b349acc462d345a6e298a76f7d06fb1 @bnoordhuis bnoordhuis committed Jun 6, 2012
Showing with 53 additions and 38 deletions.
  1. +48 −35 lib/fs.js
  2. +5 −3 test/simple/test-fs-stat.js
View
@@ -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,31 +395,31 @@ 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) {
return binding.truncate(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) {
return binding.rmdir(pathModule._makeLong(path));
};
fs.fdatasync = function(fd, callback) {
- binding.fdatasync(fd, callback || noop);
+ binding.fdatasync(fd, makeCallback(callback));
};
fs.fdatasyncSync = function(fd) {
return binding.fdatasync(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,31 +439,31 @@ 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) {
return binding.sendfile(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) {
return binding.readdir(pathModule._makeLong(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,15 +519,15 @@ 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) {
return binding.unlink(pathModule._makeLong(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,15 +603,15 @@ 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) {
return binding.fchown(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) {
@@ -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

0 comments on commit 463d6ba

Please sign in to comment.