Skip to content

Commit

Permalink
Merge pull request #130 from raszi/gh-129
Browse files Browse the repository at this point in the history 
fix #129 install process listeners safely
  • Loading branch information
@silkentrance
silkentrance authored Nov 29, 2017
2 parents fb6837c + 0eba297 commit cf037ec
Show file tree
Hide file tree
Showing 23 changed files with 291 additions and 100 deletions.
50 changes: 35 additions & 15 deletions lib/tmp.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,13 @@ const
DIR_MODE = 448 /* 0o700 */,
FILE_MODE = 384 /* 0o600 */,

EVENT = 'exit',

// this will hold the objects need to be removed on exit
_removeObjects = [];

var
_gracefulCleanup = false,
_uncaughtException = false;
_gracefulCleanup = false;

/**
* Random name generator based on crypto.
Expand Down Expand Up @@ -97,7 +98,7 @@ function _isUndefined(obj) {
*/
function _parseArguments(options, callback) {
if (typeof options == 'function') {
return [callback || {}, options];
return [{}, options];
}

if (_isUndefined(options)) {
Expand Down Expand Up @@ -460,7 +461,7 @@ function _prepareRemoveCallback(removeFunction, arg) {
* @private
*/
function _garbageCollector() {
if (_uncaughtException && !_gracefulCleanup) {
if (!_gracefulCleanup) {
return;
}

Expand Down Expand Up @@ -515,8 +516,6 @@ function isExpectedError(error, code, errno) {

/**
* Sets the graceful cleanup.
*
* Also removes the created files and directories when an uncaught exception occurs.
*/
function setGracefulCleanup() {
_gracefulCleanup = true;
Expand All @@ -526,19 +525,40 @@ const version = process.versions.node.split('.').map(function (value) {
return parseInt(value, 10);
});

if (version[0] === 0 && (version[1] < 9 || version[1] === 9 && version[2] < 5)) {
process.addListener('uncaughtException', function _uncaughtExceptionThrown(err) {
_uncaughtException = true;
_garbageCollector();
/**
* If there are multiple different versions of tmp in place, make sure that
* we recognize the old listeners.
*/
function _is_legacy_listener(listener) {
return (listener.name == '_exit' || listener.name == '_uncaughtExceptionThrown')
&& listener.toString().indexOf('_garbageCollector();') > -1;
}

throw err;
function _safely_install_listener() {
var listeners = process.listeners(EVENT);

// collect any existing listeners
var existingListeners = [];
for (var i = 0, length = listeners.length; i < length; i++) {
var lstnr = listeners[i];
if (lstnr.name == '_tmp$safe_listener' || _is_legacy_listener(lstnr)) {
if (lstnr.name != '_uncaughtExceptionThrown') existingListeners.push(lstnr);
process.removeListener(EVENT, lstnr);
}
}

process.addListener(EVENT, function _tmp$safe_listener(data) {
if (existingListeners.length) {
for (var i = 0, length = existingListeners.length; i < length; i++) {
existingListeners[i](data);
}
}
_garbageCollector();
});
}

process.addListener('exit', function _exit(code) {
if (code) _uncaughtException = true;
_garbageCollector();
});
_safely_install_listener();


/**
* Configuration options.
Expand Down
4 changes: 4 additions & 0 deletions test/assertions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,7 @@ module.exports.assertDoesNotExist = function assertDoesNotExist(name) {
assert.ok(!existsSync(name), name + ' should not exist');
};

module.exports.assertDoesNotStartWith = function assertDoesNotStartWith(s, prefix, msg) {
if (s.indexOf(prefix) == 0) assert.fail(msg || s);
};

33 changes: 26 additions & 7 deletions test/child-process.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,45 @@ var
spawn = require('child_process').spawn;


module.exports = function spawnChildProcess(configFile, cb) {
module.exports.genericChildProcess = function spawnGenericChildProcess(configFile, cb) {
var
configFilePath = path.join(__dirname, 'outband', configFile),
command_args = [path.join(__dirname, 'spawn-generic.js'), configFilePath];

// make sure that the config file exists
if (!existsSync(configFilePath))
return cb(new Error('ENOENT: configFile ' + configFilePath + ' does not exist'));

_do_spawn(command_args, cb);
};

module.exports.childProcess = function spawnChildProcess(configFile, cb) {
var
configFilePath = path.join(__dirname, 'outband', configFile),
command_args = [path.join(__dirname, 'spawn-custom.js'), configFilePath];

// make sure that the config file exists
if (!existsSync(configFilePath))
return cb(new Error('ENOENT: configFile ' + configFilePath + ' does not exist'));

_do_spawn(command_args, cb);
}

function _do_spawn(command_args, cb) {
var
node_path = process.argv[0],
command_args = [path.join(__dirname, 'spawn.js')].concat(configFile),
stdoutBufs = [],
stderrBufs = [],
child,
done = false,
stderrDone = false,
stdoutDone = false;

// make sure that the config file exists
if (!existsSync(path.join(__dirname, configFile)))
return cb(new Error('ENOENT: configFile ' + path.join(__dirname, configFile) + ' does not exist'));

// spawn doesn’t have the quoting problems that exec does,
// especially when going for Windows portability.
child = spawn(node_path, command_args);
child.stdin.end();
// TODO:we no longer support node 0.6
// Cannot use 'close' event because not on node-0.6.
function _close() {
var
Expand Down Expand Up @@ -56,7 +76,6 @@ module.exports = function spawnChildProcess(configFile, cb) {
});
}


function _bufferConcat(buffers) {
if (Buffer.concat) {
return Buffer.concat.apply(this, arguments);
Expand Down
16 changes: 8 additions & 8 deletions test/dir-sync-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ var
fs = require('fs'),
path = require('path'),
inbandStandardTests = require('./inband-standard'),
childProcess = require('./child-process'),
childProcess = require('./child-process').genericChildProcess,
assertions = require('./assertions'),
tmp = require('../lib/tmp');

Expand Down Expand Up @@ -54,15 +54,15 @@ describe('tmp', function () {
// API call standard outband tests
describe('when running standard outband tests', function () {
it('on graceful cleanup', function (done) {
childProcess('outband/graceful-dir-sync.json', function (err, stderr, stdout) {
childProcess('graceful-dir-sync.json', function (err, stderr, stdout) {
if (err) return done(err);
else if (!stderr) assert.fail('stderr expected');
else assertions.assertDoesNotExist(stdout);
done();
});
});
it('on non graceful cleanup', function (done) {
childProcess('outband/non-graceful-dir-sync.json', function (err, stderr, stdout) {
childProcess('non-graceful-dir-sync.json', function (err, stderr, stdout) {
if (err) return done(err);
else if (!stderr) assert.fail('stderr expected');
else {
Expand All @@ -73,7 +73,7 @@ describe('tmp', function () {
});
});
it('on keep', function (done) {
childProcess('outband/keep-dir-sync.json', function (err, stderr, stdout) {
childProcess('keep-dir-sync.json', function (err, stderr, stdout) {
if (err) return done(err);
else if (stderr) assert.fail(stderr);
else {
Expand All @@ -84,15 +84,15 @@ describe('tmp', function () {
});
});
it('on unlink (keep == false)', function (done) {
childProcess('outband/unlink-dir-sync.json', function (err, stderr, stdout) {
childProcess('unlink-dir-sync.json', function (err, stderr, stdout) {
if (err) return done(err);
else if (stderr) assert.fail(stderr);
else assertions.assertDoesNotExist(stdout);
done();
});
});
it('on unsafe cleanup', function (done) {
childProcess('outband/unsafe-sync.json', function (err, stderr, stdout) {
childProcess('unsafe-sync.json', function (err, stderr, stdout) {
if (err) return done(err);
else if (stderr) assert.fail(stderr);
else {
Expand All @@ -105,7 +105,7 @@ describe('tmp', function () {
});
});
it('on non unsafe cleanup', function (done) {
childProcess('outband/non-unsafe-sync.json', function (err, stderr, stdout) {
childProcess('non-unsafe-sync.json', function (err, stderr, stdout) {
if (err) return done(err);
else if (stderr) assert.fail(stderr);
else {
Expand All @@ -128,7 +128,7 @@ describe('tmp', function () {
describe('when running issue specific outband tests', function () {
// add your issue specific tests here
it('on issue #62', function (done) {
childProcess('outband/issue62-sync.json', function (err, stderr, stdout) {
childProcess('issue62-sync.json', function (err, stderr, stdout) {
if (err) return done(err);
else if (stderr) assert.fail(stderr);
else assertions.assertDoesNotExist(stdout);
Expand Down
16 changes: 8 additions & 8 deletions test/dir-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ var
fs = require('fs'),
path = require('path'),
inbandStandardTests = require('./inband-standard'),
childProcess = require('./child-process'),
childProcess = require('./child-process').genericChildProcess,
assertions = require('./assertions'),
tmp = require('../lib/tmp');

Expand Down Expand Up @@ -55,15 +55,15 @@ describe('tmp', function () {
// API call standard outband tests
describe('when running standard outband tests', function () {
it('on graceful cleanup', function (done) {
childProcess('outband/graceful-dir.json', function (err, stderr, stdout) {
childProcess('graceful-dir.json', function (err, stderr, stdout) {
if (err) return done(err);
else if (!stderr) assert.fail('stderr expected');
else assertions.assertDoesNotExist(stdout);
done();
});
});
it('on non graceful cleanup', function (done) {
childProcess('outband/non-graceful-dir.json', function (err, stderr, stdout) {
childProcess('non-graceful-dir.json', function (err, stderr, stdout) {
if (err) return done(err);
else if (!stderr) assert.fail('stderr expected');
else {
Expand All @@ -74,7 +74,7 @@ describe('tmp', function () {
});
});
it('on keep', function (done) {
childProcess('outband/keep-dir.json', function (err, stderr, stdout) {
childProcess('keep-dir.json', function (err, stderr, stdout) {
if (err) return done(err);
else if (stderr) assert.fail(stderr);
else {
Expand All @@ -85,15 +85,15 @@ describe('tmp', function () {
});
});
it('on unlink (keep == false)', function (done) {
childProcess('outband/unlink-dir.json', function (err, stderr, stdout) {
childProcess('unlink-dir.json', function (err, stderr, stdout) {
if (err) return done(err);
else if (stderr) assert.fail(stderr);
else assertions.assertDoesNotExist(stdout);
done();
});
});
it('on unsafe cleanup', function (done) {
childProcess('outband/unsafe.json', function (err, stderr, stdout) {
childProcess('unsafe.json', function (err, stderr, stdout) {
if (err) return done(err);
else if (stderr) assert.fail(stderr);
else {
Expand All @@ -106,7 +106,7 @@ describe('tmp', function () {
});
});
it('on non unsafe cleanup', function (done) {
childProcess('outband/non-unsafe.json', function (err, stderr, stdout) {
childProcess('non-unsafe.json', function (err, stderr, stdout) {
if (err) return done(err);
else if (stderr) assert.fail(stderr);
else {
Expand All @@ -129,7 +129,7 @@ describe('tmp', function () {
describe('when running issue specific outband tests', function () {
// add your issue specific tests here
it('on issue #62', function (done) {
childProcess('outband/issue62.json', function (err, stderr, stdout) {
childProcess('issue62.json', function (err, stderr, stdout) {
if (err) return done(err);
else if (stderr) assert.fail(stderr);
else assertions.assertDoesNotExist(stdout);
Expand Down
12 changes: 6 additions & 6 deletions test/file-sync-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ var
fs = require('fs'),
inbandStandardTests = require('./inband-standard'),
assertions = require('./assertions'),
childProcess = require('./child-process'),
childProcess = require('./child-process').genericChildProcess,
tmp = require('../lib/tmp');


Expand Down Expand Up @@ -51,15 +51,15 @@ describe('tmp', function () {
// API call standard outband tests
describe('when running standard outband tests', function () {
it('on graceful', function (done) {
childProcess('outband/graceful-file-sync.json', function (err, stderr, stdout) {
childProcess('graceful-file-sync.json', function (err, stderr, stdout) {
if (err) return done(err);
else if (!stderr) assert.fail('stderr expected');
else assertions.assertDoesNotExist(stdout);
done();
});
});
it('on non graceful', function (done) {
childProcess('outband/non-graceful-file-sync.json', function (err, stderr, stdout) {
childProcess('non-graceful-file-sync.json', function (err, stderr, stdout) {
if (err) return done(err);
else if (!stderr) assert.fail('stderr expected');
else {
Expand All @@ -70,7 +70,7 @@ describe('tmp', function () {
});
});
it('on keep', function (done) {
childProcess('outband/keep-file-sync.json', function (err, stderr, stdout) {
childProcess('keep-file-sync.json', function (err, stderr, stdout) {
if (err) return done(err);
else if (stderr) assert.fail(stderr);
else {
Expand All @@ -81,7 +81,7 @@ describe('tmp', function () {
});
});
it('on unlink (keep == false)', function (done) {
childProcess('outband/unlink-file-sync.json', function (err, stderr, stdout) {
childProcess('unlink-file-sync.json', function (err, stderr, stdout) {
if (err) return done(err);
else if (stderr) assert.fail(stderr);
else assertions.assertDoesNotExist(stdout);
Expand All @@ -94,7 +94,7 @@ describe('tmp', function () {
describe('when running issue specific outband tests', function () {
// add your issue specific tests here
it('on issue #115', function (done) {
childProcess('outband/issue115-sync.json', function (err, stderr, stdout) {
childProcess('issue115-sync.json', function (err, stderr, stdout) {
if (err) return done(err);
else if (stderr) assert.fail(stderr);
else assertions.assertDoesNotExist(stdout);
Expand Down
Loading

0 comments on commit cf037ec

Please sign in to comment.