From 395ccc8cf3cc119dc98a0d38940dbfaa590af0cd Mon Sep 17 00:00:00 2001 From: isaacs Date: Fri, 2 Dec 2016 13:42:29 -0800 Subject: [PATCH 1/2] fix: Allow infinite event handlers on the emitter See https://github.com/sindresorhus/execa/issues/61#issuecomment-264418946 Since the emitter is a global singleton, Node's default limit of 10 event handlers before warning is overly restrictive. It's very possible to end up with more than 10 onExit handlers, even in very unremarkable use cases. One might argue that "Infinity" is overly lenient, but then what heuristic makes sense? Probably 1000 would be suitably excessive. But I think any default other than Infinity is just "as many as you want" anyway, and the right solution (if we care about limiting it) would be to let the user set an arbitrary limit by exposing the emitter. Exposing the emitter has its own trade-offs, of course, by increasing the surface of the API and also encouraging some potentially weird behavior. --- index.js | 9 +++++++++ test/many-handlers.js | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 test/many-handlers.js diff --git a/index.js b/index.js index 7dd8d91..337f691 100644 --- a/index.js +++ b/index.js @@ -19,6 +19,15 @@ if (process.__signal_exit_emitter__) { emitter.emitted = {} } +// Because this emitter is a global, we have to check to see if a +// previous version of this library failed to enable infinite listeners. +// I know what you're about to say. But literally everything about +// signal-exit is a compromise with evil. Get used to it. +if (!emitter.infinite) { + emitter.setMaxListeners(Infinity) + emitter.infinite = true +} + module.exports = function (cb, opts) { assert.equal(typeof cb, 'function', 'a callback must be provided for exit handler') diff --git a/test/many-handlers.js b/test/many-handlers.js new file mode 100644 index 0000000..93b923c --- /dev/null +++ b/test/many-handlers.js @@ -0,0 +1,33 @@ +var onexit = require('../') +var spawn = require('child_process').spawn +var t = require('tap') +var node = process.execPath +var f = __filename + +if (process.argv[2] === 'child') { + for (var i = 0; i < 15; i++) { + onexit(function () { + console.log('ok') + }) + } +} else { + t.test('parent', function (t) { + var child = spawn(node, [f, 'child']) + var err = '' + var out = '' + var expectOut = new Array(16).join('ok\n') + child.stderr.on('data', function (c) { + err += c + }) + child.stdout.on('data', function (c) { + out += c + }) + child.on('close', function (code, signal) { + t.equal(code, 0) + t.equal(signal, null) + t.equal(err, '') + t.equal(out, expectOut) + t.end() + }) + }) +} From 9cc8eadcf748543b6a21d8e0d89c4ccce734f32c Mon Sep 17 00:00:00 2001 From: isaacs Date: Fri, 2 Dec 2016 13:48:42 -0800 Subject: [PATCH 2/2] chore: update tap for faster testing --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 8429fac..ff311e8 100644 --- a/package.json +++ b/package.json @@ -33,6 +33,6 @@ "nyc": "^8.1.0", "standard": "^7.1.2", "standard-version": "^2.3.0", - "tap": "^7.1.0" + "tap": "^8.0.1" } }