From a8d1fdcb1ce1823dc7ac13fb406e03211efa863c Mon Sep 17 00:00:00 2001 From: Chunpeng Huo Date: Mon, 27 Jul 2020 13:26:32 +1000 Subject: [PATCH] fix: fix the memory leak introduced in nodejs v10 patch closes #302 --- lib/binding.js | 168 +++++++++++++++++++++++++++---------------------- lib/index.js | 15 +---- 2 files changed, 94 insertions(+), 89 deletions(-) diff --git a/lib/binding.js b/lib/binding.js index 0e34b9e4..90b912cd 100644 --- a/lib/binding.js +++ b/lib/binding.js @@ -240,19 +240,6 @@ Stats.prototype.isSocket = function() { return this._checkModeProperty(constants.S_IFSOCK); }; -// I don't know exactly what is going on. -// If _openFiles is a property of binding instance, there is a strange -// bug in nodejs v10+ that something cleaned up this._openFiles from -// nowhere. It happens after second mockfs(), after first mockfs()+restore(). - -// So I moved _openFiles to a private var. The other two vars (_system, -// _counter) do not hurt. -// This fixed https://github.com/tschaub/mock-fs/issues/254 -// But I did not dig deep enough to understand what exactly happened. -let _system; -let _openFiles = {}; -let _counter = 0; - /** * Create a new binding with the given file system. * @param {FileSystem} system Mock file system. @@ -263,7 +250,7 @@ function Binding(system) { * Mock file system. * @type {FileSystem} */ - _system = system; + this._system = system; /** * Stats constructor. @@ -275,13 +262,13 @@ function Binding(system) { * Lookup of open files. * @type {Object.} */ - _openFiles = {}; + this._openFiles = {}; /** * Counter for file descriptors. * @type {number} */ - _counter = 0; + this._counter = 0; } /** @@ -289,7 +276,7 @@ function Binding(system) { * @return {FileSystem} The underlying file system. */ Binding.prototype.getSystem = function() { - return _system; + return this._system; }; /** @@ -297,7 +284,7 @@ Binding.prototype.getSystem = function() { * @param {FileSystem} system The new file system. */ Binding.prototype.setSystem = function(system) { - _system = system; + this._system = system; }; /** @@ -305,34 +292,34 @@ Binding.prototype.setSystem = function(system) { * @param {number} fd File descriptor identifier. * @return {FileDescriptor} File descriptor. */ -function getDescriptorById(fd) { - if (!_openFiles.hasOwnProperty(fd)) { +Binding.prototype.getDescriptorById = function(fd) { + if (!this._openFiles.hasOwnProperty(fd)) { throw new FSError('EBADF'); } - return _openFiles[fd]; -} + return this._openFiles[fd]; +}; /** * Keep track of a file descriptor as open. * @param {FileDescriptor} descriptor The file descriptor. * @return {number} Identifier for file descriptor. */ -function trackDescriptor(descriptor) { - const fd = ++_counter; - _openFiles[fd] = descriptor; +Binding.prototype.trackDescriptor = function(descriptor) { + const fd = ++this._counter; + this._openFiles[fd] = descriptor; return fd; -} +}; /** * Stop tracking a file descriptor as open. * @param {number} fd Identifier for file descriptor. */ -function untrackDescriptorById(fd) { - if (!_openFiles.hasOwnProperty(fd)) { +Binding.prototype.untrackDescriptorById = function(fd) { + if (!this._openFiles.hasOwnProperty(fd)) { throw new FSError('EBADF'); } - delete _openFiles[fd]; -} + delete this._openFiles[fd]; +}; /** * Resolve the canonicalized absolute pathname. @@ -343,20 +330,21 @@ function untrackDescriptorById(fd) { */ Binding.prototype.realpath = function(filepath, encoding, callback, ctx) { markSyscall(ctx, 'realpath'); + const self = this; return maybeCallback(normalizeCallback(callback), ctx, this, function() { let realPath; filepath = deBuffer(filepath); const resolved = path.resolve(filepath); const parts = getPathParts(resolved); - let item = _system.getRoot(); + let item = self._system.getRoot(); let itemPath = '/'; let name, i, ii; for (i = 0, ii = parts.length; i < ii; ++i) { name = parts[i]; while (item instanceof SymbolicLink) { itemPath = path.resolve(path.dirname(itemPath), item.getPath()); - item = _system.getItem(itemPath); + item = self._system.getItem(itemPath); } if (!item) { throw new FSError('ENOENT', filepath); @@ -371,7 +359,7 @@ Binding.prototype.realpath = function(filepath, encoding, callback, ctx) { if (item) { while (item instanceof SymbolicLink) { itemPath = path.resolve(path.dirname(itemPath), item.getPath()); - item = _system.getItem(itemPath); + item = self._system.getItem(itemPath); } realPath = itemPath; } else { @@ -450,12 +438,13 @@ Binding.prototype.stat = function(filepath, options, callback, ctx) { } markSyscall(ctx, 'stat'); + const self = this; return maybeCallback(wrapStatsCallback(callback), ctx, this, function() { filepath = deBuffer(filepath); - let item = _system.getItem(filepath); + let item = self._system.getItem(filepath); if (item instanceof SymbolicLink) { - item = _system.getItem( + item = self._system.getItem( path.resolve(path.dirname(filepath), item.getPath()) ); } @@ -494,9 +483,10 @@ Binding.prototype.fstat = function(fd, options, callback, ctx) { } markSyscall(ctx, 'fstat'); + const self = this; return maybeCallback(wrapStatsCallback(callback), ctx, this, function() { - const descriptor = getDescriptorById(fd); + const descriptor = self.getDescriptorById(fd); const item = descriptor.getItem(); const stats = item.getStats(); @@ -523,9 +513,10 @@ Binding.prototype.fstat = function(fd, options, callback, ctx) { */ Binding.prototype.close = function(fd, callback, ctx) { markSyscall(ctx, 'close'); + const self = this; return maybeCallback(normalizeCallback(callback), ctx, this, function() { - untrackDescriptorById(fd); + self.untrackDescriptorById(fd); }); }; @@ -540,13 +531,14 @@ Binding.prototype.close = function(fd, callback, ctx) { */ Binding.prototype.open = function(pathname, flags, mode, callback, ctx) { markSyscall(ctx, 'open'); + const self = this; return maybeCallback(normalizeCallback(callback), ctx, this, function() { pathname = deBuffer(pathname); const descriptor = new FileDescriptor(flags); - let item = _system.getItem(pathname); + let item = self._system.getItem(pathname); while (item instanceof SymbolicLink) { - item = _system.getItem( + item = self._system.getItem( path.resolve(path.dirname(pathname), item.getPath()) ); } @@ -554,7 +546,7 @@ Binding.prototype.open = function(pathname, flags, mode, callback, ctx) { throw new FSError('EEXIST', pathname); } if (descriptor.isCreate() && !item) { - const parent = _system.getItem(path.dirname(pathname)); + const parent = self._system.getItem(path.dirname(pathname)); if (!parent) { throw new FSError('ENOENT', pathname); } @@ -594,7 +586,7 @@ Binding.prototype.open = function(pathname, flags, mode, callback, ctx) { descriptor.setPosition(item.getContent().length); } descriptor.setItem(item); - return trackDescriptor(descriptor); + return self.trackDescriptor(descriptor); }); }; @@ -642,9 +634,10 @@ Binding.prototype.read = function( ctx ) { markSyscall(ctx, 'read'); + const self = this; return maybeCallback(normalizeCallback(callback), ctx, this, function() { - const descriptor = getDescriptorById(fd); + const descriptor = self.getDescriptorById(fd); if (!descriptor.isRead()) { throw new FSError('EBADF'); } @@ -679,6 +672,7 @@ Binding.prototype.read = function( */ Binding.prototype.copyFile = function(src, dest, flags, callback, ctx) { markSyscall(ctx, 'copyfile'); + const self = this; return maybeCallback(normalizeCallback(callback), ctx, this, function() { src = deBuffer(src); @@ -686,7 +680,7 @@ Binding.prototype.copyFile = function(src, dest, flags, callback, ctx) { const srcFd = this.open(src, constants.O_RDONLY); try { - const srcDescriptor = getDescriptorById(srcFd); + const srcDescriptor = self.getDescriptorById(srcFd); if (!srcDescriptor.isRead()) { throw new FSError('EBADF'); } @@ -735,9 +729,10 @@ Binding.prototype.writeBuffers = function( ctx ) { markSyscall(ctx, 'write'); + const self = this; return maybeCallback(normalizeCallback(callback), ctx, this, function() { - const descriptor = getDescriptorById(fd); + const descriptor = self.getDescriptorById(fd); if (!descriptor.isWrite()) { throw new FSError('EBADF'); } @@ -787,9 +782,10 @@ Binding.prototype.writeBuffer = function( ctx ) { markSyscall(ctx, 'write'); + const self = this; return maybeCallback(normalizeCallback(callback), ctx, this, function() { - const descriptor = getDescriptorById(fd); + const descriptor = self.getDescriptorById(fd); if (!descriptor.isWrite()) { throw new FSError('EBADF'); } @@ -880,18 +876,19 @@ Binding.prototype.writeString = function( */ Binding.prototype.rename = function(oldPath, newPath, callback, ctx) { markSyscall(ctx, 'rename'); + const self = this; return maybeCallback(normalizeCallback(callback), ctx, this, function() { oldPath = deBuffer(oldPath); newPath = deBuffer(newPath); - const oldItem = _system.getItem(oldPath); + const oldItem = self._system.getItem(oldPath); if (!oldItem) { throw new FSError('ENOENT', oldPath); } - const oldParent = _system.getItem(path.dirname(oldPath)); + const oldParent = self._system.getItem(path.dirname(oldPath)); const oldName = path.basename(oldPath); - const newItem = _system.getItem(newPath); - const newParent = _system.getItem(path.dirname(newPath)); + const newItem = self._system.getItem(newPath); + const newParent = self._system.getItem(path.dirname(newPath)); const newName = path.basename(newPath); if (newItem) { // make sure they are the same type @@ -947,14 +944,15 @@ Binding.prototype.readdir = function( } markSyscall(ctx, 'scandir'); + const self = this; return maybeCallback(normalizeCallback(callback), ctx, this, function() { dirpath = deBuffer(dirpath); let dpath = dirpath; - let dir = _system.getItem(dirpath); + let dir = self._system.getItem(dirpath); while (dir instanceof SymbolicLink) { dpath = path.resolve(path.dirname(dpath), dir.getPath()); - dir = _system.getItem(dpath); + dir = self._system.getItem(dpath); } if (!dir) { throw new FSError('ENOENT', dirpath); @@ -1003,10 +1001,11 @@ Binding.prototype.mkdir = function(pathname, mode, recursive, callback, ctx) { } markSyscall(ctx, 'mkdir'); + const self = this; return maybeCallback(normalizeCallback(callback), ctx, this, function() { pathname = deBuffer(pathname); - const item = _system.getItem(pathname); + const item = self._system.getItem(pathname); if (item) { if (recursive && item instanceof Directory) { // silently pass existing folder in recursive mode @@ -1017,7 +1016,7 @@ Binding.prototype.mkdir = function(pathname, mode, recursive, callback, ctx) { const _mkdir = function(_pathname) { const parentDir = path.dirname(_pathname); - let parent = _system.getItem(parentDir); + let parent = self._system.getItem(parentDir); if (!parent) { if (!recursive) { throw new FSError('ENOENT', _pathname); @@ -1044,10 +1043,11 @@ Binding.prototype.mkdir = function(pathname, mode, recursive, callback, ctx) { */ Binding.prototype.rmdir = function(pathname, callback, ctx) { markSyscall(ctx, 'rmdir'); + const self = this; return maybeCallback(normalizeCallback(callback), ctx, this, function() { pathname = deBuffer(pathname); - const item = _system.getItem(pathname); + const item = self._system.getItem(pathname); if (!item) { throw new FSError('ENOENT', pathname); } @@ -1058,7 +1058,7 @@ Binding.prototype.rmdir = function(pathname, callback, ctx) { throw new FSError('ENOTEMPTY', pathname); } this.access(path.dirname(pathname), parseInt('0002', 8)); - const parent = _system.getItem(path.dirname(pathname)); + const parent = self._system.getItem(path.dirname(pathname)); parent.removeItem(path.basename(pathname)); }); }; @@ -1083,11 +1083,12 @@ Binding.prototype.mkdtemp = function(prefix, encoding, callback, ctx) { } markSyscall(ctx, 'mkdtemp'); + const self = this; return maybeCallback(normalizeCallback(callback), ctx, this, function() { prefix = prefix.replace(/X{0,6}$/, 'XXXXXX'); const parentPath = path.dirname(prefix); - const parent = _system.getItem(parentPath); + const parent = self._system.getItem(parentPath); if (!parent) { throw new FSError('ENOENT', prefix); } @@ -1137,9 +1138,10 @@ Binding.prototype.mkdtemp = function(prefix, encoding, callback, ctx) { */ Binding.prototype.ftruncate = function(fd, len, callback, ctx) { markSyscall(ctx, 'ftruncate'); + const self = this; return maybeCallback(normalizeCallback(callback), ctx, this, function() { - const descriptor = getDescriptorById(fd); + const descriptor = self.getDescriptorById(fd); if (!descriptor.isWrite()) { throw new FSError('EINVAL'); } @@ -1173,10 +1175,11 @@ Binding.prototype.truncate = Binding.prototype.ftruncate; */ Binding.prototype.chown = function(pathname, uid, gid, callback, ctx) { markSyscall(ctx, 'chown'); + const self = this; return maybeCallback(normalizeCallback(callback), ctx, this, function() { pathname = deBuffer(pathname); - const item = _system.getItem(pathname); + const item = self._system.getItem(pathname); if (!item) { throw new FSError('ENOENT', pathname); } @@ -1195,9 +1198,10 @@ Binding.prototype.chown = function(pathname, uid, gid, callback, ctx) { */ Binding.prototype.fchown = function(fd, uid, gid, callback, ctx) { markSyscall(ctx, 'fchown'); + const self = this; return maybeCallback(normalizeCallback(callback), ctx, this, function() { - const descriptor = getDescriptorById(fd); + const descriptor = self.getDescriptorById(fd); const item = descriptor.getItem(); item.setUid(uid); item.setGid(gid); @@ -1213,10 +1217,11 @@ Binding.prototype.fchown = function(fd, uid, gid, callback, ctx) { */ Binding.prototype.chmod = function(pathname, mode, callback, ctx) { markSyscall(ctx, 'chmod'); + const self = this; return maybeCallback(normalizeCallback(callback), ctx, this, function() { pathname = deBuffer(pathname); - const item = _system.getItem(pathname); + const item = self._system.getItem(pathname); if (!item) { throw new FSError('ENOENT', pathname); } @@ -1233,9 +1238,10 @@ Binding.prototype.chmod = function(pathname, mode, callback, ctx) { */ Binding.prototype.fchmod = function(fd, mode, callback, ctx) { markSyscall(ctx, 'fchmod'); + const self = this; return maybeCallback(normalizeCallback(callback), ctx, this, function() { - const descriptor = getDescriptorById(fd); + const descriptor = self.getDescriptorById(fd); const item = descriptor.getItem(); item.setMode(mode); }); @@ -1249,17 +1255,18 @@ Binding.prototype.fchmod = function(fd, mode, callback, ctx) { */ Binding.prototype.unlink = function(pathname, callback, ctx) { markSyscall(ctx, 'unlink'); + const self = this; return maybeCallback(normalizeCallback(callback), ctx, this, function() { pathname = deBuffer(pathname); - const item = _system.getItem(pathname); + const item = self._system.getItem(pathname); if (!item) { throw new FSError('ENOENT', pathname); } if (item instanceof Directory) { throw new FSError('EPERM', pathname); } - const parent = _system.getItem(path.dirname(pathname)); + const parent = self._system.getItem(path.dirname(pathname)); parent.removeItem(path.basename(pathname)); }); }; @@ -1274,10 +1281,11 @@ Binding.prototype.unlink = function(pathname, callback, ctx) { */ Binding.prototype.utimes = function(pathname, atime, mtime, callback, ctx) { markSyscall(ctx, 'utimes'); + const self = this; return maybeCallback(normalizeCallback(callback), ctx, this, function() { pathname = deBuffer(pathname); - const item = _system.getItem(pathname); + const item = self._system.getItem(pathname); if (!item) { throw new FSError('ENOENT', pathname); } @@ -1296,9 +1304,10 @@ Binding.prototype.utimes = function(pathname, atime, mtime, callback, ctx) { */ Binding.prototype.futimes = function(fd, atime, mtime, callback, ctx) { markSyscall(ctx, 'futimes'); + const self = this; return maybeCallback(normalizeCallback(callback), ctx, this, function() { - const descriptor = getDescriptorById(fd); + const descriptor = self.getDescriptorById(fd); const item = descriptor.getItem(); item.setATime(new Date(atime * 1000)); item.setMTime(new Date(mtime * 1000)); @@ -1313,9 +1322,10 @@ Binding.prototype.futimes = function(fd, atime, mtime, callback, ctx) { */ Binding.prototype.fsync = function(fd, callback, ctx) { markSyscall(ctx, 'fsync'); + const self = this; return maybeCallback(normalizeCallback(callback), ctx, this, function() { - getDescriptorById(fd); + self.getDescriptorById(fd); }); }; @@ -1327,9 +1337,10 @@ Binding.prototype.fsync = function(fd, callback, ctx) { */ Binding.prototype.fdatasync = function(fd, callback, ctx) { markSyscall(ctx, 'fdatasync'); + const self = this; return maybeCallback(normalizeCallback(callback), ctx, this, function() { - getDescriptorById(fd); + self.getDescriptorById(fd); }); }; @@ -1342,21 +1353,22 @@ Binding.prototype.fdatasync = function(fd, callback, ctx) { */ Binding.prototype.link = function(srcPath, destPath, callback, ctx) { markSyscall(ctx, 'link'); + const self = this; return maybeCallback(normalizeCallback(callback), ctx, this, function() { srcPath = deBuffer(srcPath); destPath = deBuffer(destPath); - const item = _system.getItem(srcPath); + const item = self._system.getItem(srcPath); if (!item) { throw new FSError('ENOENT', srcPath); } if (item instanceof Directory) { throw new FSError('EPERM', srcPath); } - if (_system.getItem(destPath)) { + if (self._system.getItem(destPath)) { throw new FSError('EEXIST', destPath); } - const parent = _system.getItem(path.dirname(destPath)); + const parent = self._system.getItem(path.dirname(destPath)); if (!parent) { throw new FSError('ENOENT', destPath); } @@ -1377,14 +1389,15 @@ Binding.prototype.link = function(srcPath, destPath, callback, ctx) { */ Binding.prototype.symlink = function(srcPath, destPath, type, callback, ctx) { markSyscall(ctx, 'symlink'); + const self = this; return maybeCallback(normalizeCallback(callback), ctx, this, function() { srcPath = deBuffer(srcPath); destPath = deBuffer(destPath); - if (_system.getItem(destPath)) { + if (self._system.getItem(destPath)) { throw new FSError('EEXIST', destPath); } - const parent = _system.getItem(path.dirname(destPath)); + const parent = self._system.getItem(path.dirname(destPath)); if (!parent) { throw new FSError('ENOENT', destPath); } @@ -1413,10 +1426,11 @@ Binding.prototype.readlink = function(pathname, encoding, callback, ctx) { } markSyscall(ctx, 'readlink'); + const self = this; return maybeCallback(normalizeCallback(callback), ctx, this, function() { pathname = deBuffer(pathname); - const link = _system.getItem(pathname); + const link = self._system.getItem(pathname); if (!link) { throw new FSError('ENOENT', pathname); } @@ -1447,10 +1461,11 @@ Binding.prototype.lstat = function(filepath, options, callback, ctx) { } markSyscall(ctx, 'lstat'); + const self = this; return maybeCallback(wrapStatsCallback(callback), ctx, this, function() { filepath = deBuffer(filepath); - const item = _system.getItem(filepath); + const item = self._system.getItem(filepath); if (!item) { throw new FSError('ENOENT', filepath); } @@ -1480,17 +1495,18 @@ Binding.prototype.lstat = function(filepath, options, callback, ctx) { */ Binding.prototype.access = function(filepath, mode, callback, ctx) { markSyscall(ctx, 'access'); + const self = this; return maybeCallback(normalizeCallback(callback), ctx, this, function() { filepath = deBuffer(filepath); - let item = _system.getItem(filepath); + let item = self._system.getItem(filepath); let links = 0; while (item instanceof SymbolicLink) { if (links > MAX_LINKS) { throw new FSError('ELOOP', filepath); } filepath = path.resolve(path.dirname(filepath), item.getPath()); - item = _system.getItem(filepath); + item = self._system.getItem(filepath); ++links; } if (!item) { diff --git a/lib/index.js b/lib/index.js index 04ac665a..6768de7c 100644 --- a/lib/index.js +++ b/lib/index.js @@ -32,7 +32,7 @@ function patch(key) { const existingMethod = realBinding[key]; realBinding[key] = function() { if (this._mockedBinding) { - return this._mockedBinding[key].apply(this, arguments); + return this._mockedBinding[key].apply(this._mockedBinding, arguments); } else { return existingMethod.apply(this, arguments); } @@ -50,17 +50,6 @@ for (const key in Binding.prototype) { function overrideBinding(binding) { realBinding._mockedBinding = binding; - - for (const key in binding) { - if (typeof realBinding[key] === 'function') { - // Stats and StatWatcher are constructors - if (key === 'Stats' || key === 'StatWatcher') { - realBinding[key] = binding[key]; - } - } else if (typeof realBinding[key] === 'undefined') { - realBinding[key] = binding[key]; - } - } } function overrideProcess(cwd, chdir) { @@ -154,7 +143,7 @@ exports = module.exports = function mock(config, options) { */ exports.getMockRoot = function() { if (realBinding._mockedBinding) { - return realBinding.getSystem().getRoot(); + return realBinding._mockedBinding.getSystem().getRoot(); } else { return {}; }