Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix #121 #159

Merged
merged 4 commits into from Mar 19, 2019
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion .eslintrc.js
Expand Up @@ -23,6 +23,7 @@ module.exports = {
"semi": [
"error",
"always"
]
],
"no-extra-boolean-cast": 0
}
};
1 change: 0 additions & 1 deletion .travis.yml
Expand Up @@ -4,7 +4,6 @@ node_js:
- "10"
- "9"
- "8"
- "6"
sudo: false
cache:
directories:
Expand Down
2 changes: 1 addition & 1 deletion appveyor.yml
Expand Up @@ -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
Expand Down
92 changes: 69 additions & 23 deletions lib/tmp.js
Expand Up @@ -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 = [];
Expand Down Expand Up @@ -101,7 +103,7 @@ function _isUndefined(obj) {
*/
function _parseArguments(options, callback) {
/* istanbul ignore else */
if (typeof options == 'function') {
if (typeof options === 'function') {
return [{}, options];
}

Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}

/**
Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -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"
}
}
34 changes: 18 additions & 16 deletions test/child-process.js
Expand Up @@ -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,
Expand All @@ -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);
}
Expand All @@ -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) {
Expand All @@ -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;
}, []));
}

5 changes: 3 additions & 2 deletions test/dir-sync-test.js
Expand Up @@ -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);
Expand Down
54 changes: 54 additions & 0 deletions 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));
silkentrance marked this conversation as resolved.
Show resolved Hide resolved

try {
if (expectExists) {
assertions.assertExists(stdout);
}
else {
assertions.assertDoesNotExist(stdout);
}
done();
} catch (err) {
done(err);
} finally {
// cleanup
if (expectExists) {
rimraf.sync(stdout);
}
}
}, signal);
};
}
34 changes: 34 additions & 0 deletions 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();
});
});
});
});
19 changes: 19 additions & 0 deletions 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);
};
4 changes: 4 additions & 0 deletions test/outband/issue121.json
@@ -0,0 +1,4 @@
{
"graceful": true,
"tc": "issue121"
}