Skip to content
New issue

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

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

Already on GitHub? # to your account

fix #155 #161

Merged
merged 1 commit into from
Oct 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@ node_modules/
coverage/
.idea/
.*.swp
package-lock.json
1 change: 1 addition & 0 deletions appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
environment:
matrix:
- nodejs_version: "6"
- nodejs_version: "7"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, how did I miss this?

- nodejs_version: "8"
- nodejs_version: "9"
- nodejs_version: "10"
Expand Down
205 changes: 111 additions & 94 deletions lib/tmp.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const crypto = require('crypto');
const _c = fs.constants && os.constants ?
{ fs: fs.constants, os: os.constants } :
process.binding('constants');
const rimraf = require('rimraf');

/*
* The working inner variables.
Expand Down Expand Up @@ -302,45 +303,6 @@ function fileSync(options) {
};
}

/**
* Removes files and folders in a directory recursively.
*
* @param {string} root
* @private
*/
function _rmdirRecursiveSync(root) {
const dirs = [root];

do {
var
dir = dirs.pop(),
deferred = false,
files = fs.readdirSync(dir);

for (var i = 0, length = files.length; i < length; i++) {
var
file = path.join(dir, files[i]),
stat = fs.lstatSync(file); // lstat so we don't recurse into symlinked directories

if (stat.isDirectory()) {
/* istanbul ignore else */
if (!deferred) {
deferred = true;
dirs.push(dir);
}
dirs.push(file);
} else {
fs.unlinkSync(file);
}
}

/* istanbul ignore else */
if (!deferred) {
fs.rmdirSync(dir);
}
} while (dirs.length !== 0);
}

/**
* Creates a temporary directory.
*
Expand Down Expand Up @@ -390,52 +352,94 @@ function dirSync(options) {
}

/**
* Prepares the callback for removal of the temporary file.
* Removes files asynchronously.
*
* @param {string} name the path of the file
* @param {number} fd file descriptor
* @param {Object} opts
* @returns {fileCallback}
* @param {Object} fdPath
* @param {Function} next
* @private
*/
function _prepareTmpFileRemoveCallback(name, fd, opts) {
const removeCallback = _prepareRemoveCallback(function _removeCallback(fdPath) {
try {
/* istanbul ignore else */
if (0 <= fdPath[0]) {
fs.closeSync(fdPath[0]);
}
}
catch (e) {
// under some node/windows related circumstances, a temporary file
// may have not be created as expected or the file was already closed
// by the user, in which case we will simply ignore the error
/* istanbul ignore else */
if (!isEBADF(e) && !isENOENT(e)) {
// reraise any unanticipated error
throw e;
}
function _removeFileAsync(fdPath, next) {
const _handler = function (err) {
if (err && !isENOENT(err)) {
// reraise any unanticipated error
return next(err);
}
next();
}

if (0 <= fdPath[0])
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is too dense and error prone. Please put parentheses around the branches.

fs.close(fdPath[0], function (err) {
fs.unlink(fdPath[1], _handler);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we should extract this to a local function because it is also used in the else branch.

const _removeFile = function () { fs.unlink(fdPath[1], _handler); }

});
else fs.unlink(fdPath[1], _handler);
}

/**
* Removes files synchronously.
*
* @param {Object} fdPath
* @private
*/
function _removeFileSync(fdPath) {
try {
if (0 <= fdPath[0]) fs.closeSync(fdPath[0]);
} catch (e) {
// reraise any unanticipated error
if (!isEBADF(e) && !isENOENT(e)) throw e;
} finally {
try {
fs.unlinkSync(fdPath[1]);
}
catch (e) {
/* istanbul ignore else */
if (!isENOENT(e)) {
// reraise any unanticipated error
throw e;
}
// reraise any unanticipated error
if (!isENOENT(e)) throw e;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will cause a linter error because of no-unsafe-finally.

}
}, [fd, name]);

/* istanbul ignore else */
if (!opts.keep) {
_removeObjects.unshift(removeCallback);
}
}

/**
* Prepares the callback for removal of the temporary file.
*
* @param {string} name the path of the file
* @param {number} fd file descriptor
* @param {Object} opts
* @returns {fileCallback}
* @private
*/
function _prepareTmpFileRemoveCallback(name, fd, opts) {
const removeCallbackSync = _prepareRemoveCallback(_removeFileSync, [fd, name]);
const removeCallback = _prepareRemoveCallback(_removeFileAsync, [fd, name], removeCallbackSync);

if (!opts.keep) _removeObjects.unshift(removeCallbackSync);

return removeCallback;
}

/**
* Simple wrapper for rimraf.
*
* @param {string} dirPath
* @param {Function} next
* @private
*/
function _rimrafRemoveDirWrapper(dirPath, next) {
rimraf(dirPath, next);
}

/**
* Simple wrapper for rimraf.sync.
*
* @param {string} dirPath
* @private
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The next parameter is not documented.

*/
function _rimrafRemoveDirSyncWrapper(dirPath, next) {
try {
return next(null, rimraf.sync(dirPath));
} catch (err) {
return next(err);
}
}

/**
* Prepares the callback for removal of the temporary directory.
*
Expand All @@ -445,13 +449,11 @@ function _prepareTmpFileRemoveCallback(name, fd, opts) {
* @private
*/
function _prepareTmpDirRemoveCallback(name, opts) {
const removeFunction = opts.unsafeCleanup ? _rmdirRecursiveSync : fs.rmdirSync.bind(fs);
const removeCallback = _prepareRemoveCallback(removeFunction, name);

/* istanbul ignore else */
if (!opts.keep) {
_removeObjects.unshift(removeCallback);
}
const removeFunction = opts.unsafeCleanup ? _rimrafRemoveDirWrapper : fs.rmdir.bind(fs);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is technically calling rimraf. Why do we need a wrapper for this?

const removeFunctionSync = opts.unsafeCleanup ? _rimrafRemoveDirSyncWrapper : fs.rmdirSync.bind(fs);
const removeCallbackSync = _prepareRemoveCallback(removeFunctionSync, name);
const removeCallback = _prepareRemoveCallback(removeFunction, name, removeCallbackSync);
if (!opts.keep) _removeObjects.unshift(removeCallbackSync);

return removeCallback;
}
Expand All @@ -464,24 +466,32 @@ function _prepareTmpDirRemoveCallback(name, opts) {
* @returns {Function}
* @private
*/
function _prepareRemoveCallback(removeFunction, arg) {
function _prepareRemoveCallback(removeFunction, arg, cleanupCallbackSync) {
var called = false;

return function _cleanupCallback(next) {
/* istanbul ignore else */
next = next || function () {};
if (!called) {
const index = _removeObjects.indexOf(_cleanupCallback);
const toRemove = cleanupCallbackSync || _cleanupCallback;
const index = _removeObjects.indexOf(toRemove);
/* istanbul ignore else */
if (index >= 0) {
_removeObjects.splice(index, 1);
}
if (index >= 0) _removeObjects.splice(index, 1);

called = true;
removeFunction(arg);
}

/* istanbul ignore else */
if (next) next(null);
// sync?
if (removeFunction.length == 1) {
try {
removeFunction(arg);
return next(null);
}
catch (err) {
// if no next is provided and since we are
// in silent cleanup mode on process exit,
// we will ignore the error
return next(err);
}
} else return removeFunction(arg, next);
} else return next(new Error('cleanup callback has already been called'));
};
}

Expand All @@ -492,15 +502,13 @@ function _prepareRemoveCallback(removeFunction, arg) {
*/
function _garbageCollector() {
/* istanbul ignore else */
if (!_gracefulCleanup) {
return;
}
if (!_gracefulCleanup) return;

// the function being called removes itself from _removeObjects,
// loop until _removeObjects is empty
while (_removeObjects.length) {
try {
_removeObjects[0].call(null);
_removeObjects[0]();
} catch (e) {
// already removed?
}
Expand Down Expand Up @@ -555,12 +563,21 @@ function setGracefulCleanup() {
/**
* If there are multiple different versions of tmp in place, make sure that
* we recognize the old listeners.
*
* @param {Function} listener
* @private
* @returns {Boolean} true whether listener is a legacy listener
*/
function _is_legacy_listener(listener) {
return (listener.name == '_exit' || listener.name == '_uncaughtExceptionThrown')
&& listener.toString().indexOf('_garbageCollector();') > -1;
}

/**
* Safely install process exit listeners.
*
* @private
*/
function _safely_install_listener() {
var listeners = process.listeners(EVENT);

Expand All @@ -570,7 +587,7 @@ function _safely_install_listener() {
var lstnr = listeners[i];
/* istanbul ignore else */
if (lstnr.name == '_tmp$safe_listener' || _is_legacy_listener(lstnr)) {
/* istanbul ignore else */
// we must forget about the uncaughtException listener
if (lstnr.name != '_uncaughtExceptionThrown') existingListeners.push(lstnr);
process.removeListener(EVENT, lstnr);
}
Expand Down
Loading