diff --git a/.eslintrc.js b/.eslintrc.js index 45d1d61..43ed1a1 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -23,6 +23,7 @@ module.exports = { "semi": [ "error", "always" - ] + ], + "no-extra-boolean-cast": 0 } }; diff --git a/.travis.yml b/.travis.yml index fb0b545..f3dde6f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,7 +4,6 @@ node_js: - "10" - "9" - "8" - - "6" sudo: false cache: directories: diff --git a/appveyor.yml b/appveyor.yml index 15a83a7..743d881 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -2,11 +2,11 @@ environment: matrix: - - nodejs_version: "6" - nodejs_version: "7" - nodejs_version: "8" - nodejs_version: "9" - nodejs_version: "10" + - nodejs_version: "11" install: - ps: Install-Product node $env:nodejs_version diff --git a/lib/tmp.js b/lib/tmp.js index d8826bb..33d59bd 100644 --- a/lib/tmp.js +++ b/lib/tmp.js @@ -43,7 +43,9 @@ const DIR_MODE = 448 /* 0o700 */, FILE_MODE = 384 /* 0o600 */, - EVENT = 'exit', + EXIT = 'exit', + + SIGINT = 'SIGINT', // this will hold the objects need to be removed on exit _removeObjects = []; @@ -101,7 +103,7 @@ function _isUndefined(obj) { */ function _parseArguments(options, callback) { /* istanbul ignore else */ - if (typeof options == 'function') { + if (typeof options === 'function') { return [{}, options]; } @@ -132,7 +134,7 @@ function _generateTmpName(opts) { var template = opts.template; // make sure that we prepend the tmp path if none was given /* istanbul ignore else */ - if (path.basename(template) == template) + if (path.basename(template) === template) template = path.join(opts.dir || tmpDir, template); return template.replace(TEMPLATE_PATTERN, _randomChars(6)); } @@ -479,7 +481,7 @@ function _prepareRemoveCallback(removeFunction, arg, cleanupCallbackSync) { called = true; // sync? - if (removeFunction.length == 1) { + if (removeFunction.length === 1) { try { removeFunction(arg); return next(null); @@ -550,7 +552,7 @@ function isENOENT(error) { * error.errno n/a */ function isExpectedError(error, code, errno) { - return error.code == code || error.code == errno; + return error.code === code || error.code === errno; } /** @@ -569,43 +571,87 @@ function setGracefulCleanup() { * @returns {Boolean} true whether listener is a legacy listener */ function _is_legacy_listener(listener) { - return (listener.name == '_exit' || listener.name == '_uncaughtExceptionThrown') + return (listener.name === '_exit' || listener.name === '_uncaughtExceptionThrown') && listener.toString().indexOf('_garbageCollector();') > -1; } /** - * Safely install process exit listeners. - * + * Safely install SIGINT listener. + * + * NOTE: this will only work on OSX and Linux. + * * @private */ -function _safely_install_listener() { - var listeners = process.listeners(EVENT); +function _safely_install_sigint_listener() { - // collect any existing listeners - var existingListeners = []; - for (var i = 0, length = listeners.length; i < length; i++) { - var lstnr = listeners[i]; + const listeners = process.listeners(SIGINT); + const existingListeners = []; + for (let i = 0, length = listeners.length; i < length; i++) { + const lstnr = listeners[i]; /* istanbul ignore else */ - if (lstnr.name == '_tmp$safe_listener' || _is_legacy_listener(lstnr)) { - // we must forget about the uncaughtException listener - if (lstnr.name != '_uncaughtExceptionThrown') existingListeners.push(lstnr); - process.removeListener(EVENT, lstnr); + if (lstnr.name === '_tmp$sigint_listener') { + existingListeners.push(lstnr); + process.removeListener(SIGINT, lstnr); } } + process.on(SIGINT, function _tmp$sigint_listener(doExit) { + for (let i = 0, length = existingListeners.length; i < length; i++) { + // let the existing listener do the garbage collection (e.g. jest sandbox) + try { + existingListeners[i](false); + } catch (err) { + // ignore + } + } + try { + // force the garbage collector even it is called again in the exit listener + _garbageCollector(); + } finally { + if (!!doExit) { + process.exit(0); + } + } + }); +} - process.addListener(EVENT, function _tmp$safe_listener(data) { +/** + * Safely install process exit listener. + * + * @private + */ +function _safely_install_exit_listener() { + const listeners = process.listeners(EXIT); + + // collect any existing listeners + const existingListeners = []; + for (let i = 0, length = listeners.length; i < length; i++) { + const lstnr = listeners[i]; /* istanbul ignore else */ - if (existingListeners.length) { - for (var i = 0, length = existingListeners.length; i < length; i++) { + // TODO: remove support for legacy listeners once release 1.0.0 is out + if (lstnr.name === '_tmp$safe_listener' || _is_legacy_listener(lstnr)) { + // we must forget about the uncaughtException listener, hopefully it is ours + if (lstnr.name !== '_uncaughtExceptionThrown') { + existingListeners.push(lstnr); + } + process.removeListener(EXIT, lstnr); + } + } + // TODO: what was the data parameter good for? + process.addListener(EXIT, function _tmp$safe_listener(data) { + for (let i = 0, length = existingListeners.length; i < length; i++) { + // let the existing listener do the garbage collection (e.g. jest sandbox) + try { existingListeners[i](data); + } catch (err) { + // ignore } } _garbageCollector(); }); } -_safely_install_listener(); - +_safely_install_exit_listener(); +_safely_install_sigint_listener(); /** * Configuration options. diff --git a/package.json b/package.json index fd60e8c..0c178bc 100644 --- a/package.json +++ b/package.json @@ -38,7 +38,7 @@ "scripts": { "lint": "eslint lib --env mocha test", "clean": "rm -Rf ./coverage", - "test": "istanbul cover ./node_modules/mocha/bin/_mocha --report none --print none --dir ./coverage/json -- -u exports test/*-test.js && istanbul report --root ./coverage/json html && istanbul report text-summary", + "test": "npm run clean && istanbul cover ./node_modules/mocha/bin/_mocha --report none --print none --dir ./coverage/json -u exports -R test/*-test.js && istanbul report --root ./coverage/json html && istanbul report text-summary", "doc": "jsdoc -c .jsdoc.json" } } diff --git a/test/child-process.js b/test/child-process.js index 3dd0dde..dd9fd5b 100644 --- a/test/child-process.js +++ b/test/child-process.js @@ -12,26 +12,25 @@ module.exports.genericChildProcess = _spawnProcess('spawn-generic.js'); module.exports.childProcess = _spawnProcess('spawn-custom.js'); function _spawnProcess(spawnFile) { - return function (testCase, configFile, cb) { - testCase.timeout(5000); - - var + return function (testCase, configFile, cb, signal) { + const configFilePath = path.join(__dirname, 'outband', configFile), commandArgs = [path.join(__dirname, spawnFile), configFilePath]; exists(configFilePath, function (configExists) { - if (configExists) return _doSpawn(commandArgs, cb); + if (configExists) return _doSpawn(commandArgs, cb, signal); cb(new Error('ENOENT: configFile ' + configFilePath + ' does not exist')); }); }; } -function _doSpawn(commandArgs, cb) { - var +function _doSpawn(commandArgs, cb, signal) { + const node_path = process.argv[0], stdoutBufs = [], - stderrBufs = [], + stderrBufs = []; + let child, done = false, stderrDone = false, @@ -50,14 +49,11 @@ function _doSpawn(commandArgs, cb) { child = spawn(node_path, commandArgs); child.stdin.end(); - // TODO we no longer support node 0.6 - // Cannot use 'close' event because not on node-0.6. function _close() { - var - stderr = _bufferConcat(stderrBufs).toString(), - stdout = _bufferConcat(stdoutBufs).toString(); - if (stderrDone && stdoutDone && !done) { + const + stderr = _bufferConcat(stderrBufs).toString(), + stdout = _bufferConcat(stdoutBufs).toString(); done = true; cb(null, stderr, stdout); } @@ -83,6 +79,13 @@ function _doSpawn(commandArgs, cb) { stderrDone = true; _close(); }); + + if (signal) { + setTimeout(function () { + // does not work on node 6 + child.kill(signal); + }, 2000); + } } function _bufferConcat(buffers) { @@ -91,10 +94,9 @@ function _bufferConcat(buffers) { } return new Buffer(buffers.reduce(function (acc, buf) { - for (var i = 0; i < buf.length; i++) { + for (let i = 0; i < buf.length; i++) { acc.push(buf[i]); } return acc; }, [])); } - diff --git a/test/dir-sync-test.js b/test/dir-sync-test.js index 343e442..7afe4cc 100644 --- a/test/dir-sync-test.js +++ b/test/dir-sync-test.js @@ -125,10 +125,11 @@ describe('tmp', function () { try { assertions.assertExists(stdout); assertions.assertExists(path.join(stdout, 'should-be-removed.file'), true); - if (process.platform == 'win32') + if (process.platform == 'win32') { assertions.assertExists(path.join(stdout, 'symlinkme-target'), true); - else + } else { assertions.assertExists(path.join(stdout, 'symlinkme-target')); + } } catch (err) { rimraf.sync(stdout); return done(err); diff --git a/test/issue121-test.js b/test/issue121-test.js new file mode 100644 index 0000000..4eaddcb --- /dev/null +++ b/test/issue121-test.js @@ -0,0 +1,54 @@ +/* eslint-disable no-octal */ +// vim: expandtab:ts=2:sw=2 + +const + assertions = require('./assertions'), + childProcess = require('./child-process').childProcess, + os = require('os'), + rimraf = require('rimraf'), + testCases = [ + { signal: 'SIGINT', expectExists: false }, + { signal: 'SIGTERM', expectExists: true } + ]; + +// skip tests on win32 +const isWindows = os.platform() === 'win32'; +const tfunc = isWindows ? xit : it; + +describe('tmp', function () { + describe('issue121 - clean up on terminating signals', function () { + for (let tc of testCases) { + tfunc('for signal ' + tc.signal, function (done) { + // increase timeout so that the child process may fail + this.timeout(20000); + issue121Tests(tc.signal, tc.expectExists)(done); + }); + } + }); +}); + +function issue121Tests(signal, expectExists) { + return function (done) { + childProcess(this, 'issue121.json', function (err, stderr, stdout) { + if (err) return done(err); + else if (stderr) return done(new Error(stderr)); + + try { + if (expectExists) { + assertions.assertExists(stdout); + } + else { + assertions.assertDoesNotExist(stdout); + } + done(); + } catch (err) { + done(err); + } finally { + // cleanup + if (expectExists) { + rimraf.sync(stdout); + } + } + }, signal); + }; +} diff --git a/test/issue129-sigint-test.js b/test/issue129-sigint-test.js new file mode 100644 index 0000000..0ba9b82 --- /dev/null +++ b/test/issue129-sigint-test.js @@ -0,0 +1,34 @@ +/* eslint-disable no-octal */ +// vim: expandtab:ts=2:sw=2 + +var + assert = require('assert'), + assertions = require('./assertions'), + childProcess = require('./child-process').childProcess; + +describe('tmp', function () { + describe('issue129: safely install sigint listeners', function () { + it('when simulating sandboxed behavior', function (done) { + childProcess(this, 'issue129-sigint.json', function (err, stderr, stdout) { + if (err) return done(err); + if (!stdout && !stderr) return done(new Error('stderr or stdout expected')); + if (stderr) { + try { + assertions.assertDoesNotStartWith(stderr, 'EEXISTS:MULTIPLE'); + assertions.assertDoesNotStartWith(stderr, 'ENOAVAIL:EXISTING'); + } catch (err) { + return done(err); + } + } + if (stdout) { + try { + assert.equal(stdout, 'EOK', 'existing listeners should have been removed and called'); + } catch (err) { + return done(err); + } + } + done(); + }); + }); + }); +}); diff --git a/test/outband/issue121.js b/test/outband/issue121.js new file mode 100644 index 0000000..deba395 --- /dev/null +++ b/test/outband/issue121.js @@ -0,0 +1,19 @@ +/* eslint-disable no-octal */ +// vim: expandtab:ts=2:sw=2 + +const + tmp = require('../../lib/tmp'); + +// https://github.com/raszi/node-tmp/issues/121 +module.exports = function () { + + tmp.setGracefulCleanup(); + + const result = tmp.dirSync({ unsafeCleanup: true }); + + this.out(result.name, function () { }); + + setTimeout(function () { + throw new Error('ran into timeout'); + }, 10000); +}; diff --git a/test/outband/issue121.json b/test/outband/issue121.json new file mode 100644 index 0000000..2210135 --- /dev/null +++ b/test/outband/issue121.json @@ -0,0 +1,4 @@ +{ + "graceful": true, + "tc": "issue121" +} diff --git a/test/outband/issue129-sigint.js b/test/outband/issue129-sigint.js new file mode 100644 index 0000000..0710536 --- /dev/null +++ b/test/outband/issue129-sigint.js @@ -0,0 +1,38 @@ +/* eslint-disable no-octal */ +// vim: expandtab:ts=2:sw=2 + +// addendum to https://github.com/raszi/node-tmp/issues/129 so that with jest sandboxing we do not install our sigint +// listener multiple times +module.exports = function () { + var callState = { + existingListener : false, + }; + + // simulate an existing SIGINT listener + var listener1 = (function (callState) { + return function _tmp$sigint_listener(doExit) { + callState.existingListener = !doExit; + }; + })(callState); + + process.addListener('SIGINT', listener1); + + // now let tmp install its listener safely + require('../../lib/tmp'); + + var sigintListeners = []; + + var listeners = process.listeners('SIGINT'); + for (var i = 0; i < listeners.length; i++) { + var listener = listeners[i]; + if (listener.name === '_tmp$sigint_listener') { + sigintListeners.push(listener); + } + } + + if (listeners.length > 1) this.fail('EEXISTS:MULTIPLE: existing SIGINT listener was not removed', this.exit); + listeners[0](false); // prevent listener from exiting the process + if (!callState.existingListener) this.fail('ENOAVAIL:EXISTING: existing listener was not called', this.exit); + this.out('EOK', this.exit); + process.exit(0); +}; diff --git a/test/outband/issue129-sigint.json b/test/outband/issue129-sigint.json new file mode 100644 index 0000000..967ff91 --- /dev/null +++ b/test/outband/issue129-sigint.json @@ -0,0 +1,3 @@ +{ + "tc": "issue129-sigint" +} diff --git a/test/outband/issue129.js b/test/outband/issue129.js index 0d65547..a36c56b 100644 --- a/test/outband/issue129.js +++ b/test/outband/issue129.js @@ -5,10 +5,10 @@ module.exports = function () { // dup from lib/tmp.js function _is_legacy_listener(listener) { - return (listener.name == '_exit' || listener.name == '_uncaughtExceptionThrown') - && listener.toString().indexOf('_garbageCollector();') != -1; + return (listener.name === '_exit' || listener.name === '_uncaughtExceptionThrown') + && listener.toString().indexOf('_garbageCollector();') !== -1; } - +å function _garbageCollector() {} var callState = { @@ -56,14 +56,14 @@ module.exports = function () { for (var i = 0; i < listeners.length; i++) { var listener = listeners[i]; // the order here is important - if (listener.name == '_tmp$safe_listener') { + if (listener.name === '_tmp$safe_listener') { newStyleListeners.push(listener); } else if (_is_legacy_listener(listener)) { - if (listener.name == '_uncaughtExceptionThrown') legacyUncaughtListener = listener; + if (listener.name === '_uncaughtExceptionThrown') legacyUncaughtListener = listener; else legacyExitListener = listener; } - } + }å if (legacyExitListener) this.fail('EEXISTS:LEGACY:EXIT existing legacy exit listener was not removed', this.exit); if (legacyUncaughtListener) this.fail('EEXISTS:LEGACY:UNCAUGHT existing legacy uncaught exception thrown listener was not removed', this.exit); diff --git a/test/spawn-custom.js b/test/spawn-custom.js index bb1cc27..f833f07 100644 --- a/test/spawn-custom.js +++ b/test/spawn-custom.js @@ -11,4 +11,3 @@ spawn.graceful = !!config.graceful; // import the test case function and execute it var fn = require(path.join(__dirname, 'outband', config.tc)); fn.apply(spawn); - diff --git a/test/spawn-generic.js b/test/spawn-generic.js index 6b4cb5b..d557db0 100644 --- a/test/spawn-generic.js +++ b/test/spawn-generic.js @@ -23,7 +23,7 @@ if (config.async) fnUnderTest(config.options, function (err, name, fdOrCallback, cb) { if (err) spawn.err(err); else { - var result = null; + var result = null; if (config.file) result = { name: name, fd: fdOrCallback, removeCallback: cb }; else result = { name: name, removeCallback: fdOrCallback }; fn.apply(spawn, [result, tmp]); diff --git a/test/spawn.js b/test/spawn.js index 5dfd87e..3f89510 100644 --- a/test/spawn.js +++ b/test/spawn.js @@ -28,9 +28,6 @@ module.exports = { }, exit: function (code) { process.exit(code || 0); - }, - kill: function (signal) { - process.kill(signal || 'SIGINT'); } };