rebase to master

fix order by which tests rimraf their leftovers

Author: silkentrance

.gitignore

@@ -2,3 +2,4 @@ node_modules/
 coverage/
 .idea/
 .*.swp
+package-lock.json

appveyor.yml

@@ -6,6 +6,8 @@ environment:
     - nodejs_version: "5"
     - nodejs_version: "6"
     - nodejs_version: "7"
+    - nodejs_version: "8"
+    - nodejs_version: "9"
 
 install:
   - ps: Install-Product node $env:nodejs_version

lib/tmp.js

@@ -14,9 +14,8 @@ const os = require('os');
 const path = require('path');
 const crypto = require('crypto');
 const osTmpDir = require('os-tmpdir');
-const _c = fs.constants && os.constants ?
-  { fs: fs.constants, os: os.constants } :
-  process.binding('constants');
+const rimraf = require('rimraf');
+const _c = process.binding('constants');
 
 /*
  * The working inner variables.
@@ -303,45 +302,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.
  *
@@ -391,52 +351,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])
+    fs.close(fdPath[0], function (err) {
+      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;
     }
-  }, [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
+ */
+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.
  *
@@ -446,13 +448,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);
+  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;
 }
@@ -465,24 +465,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'));
   };
 }
 
@@ -493,15 +501,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?
     }
@@ -556,12 +562,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);
 
@@ -571,7 +586,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);
     }