From 509e3a796fe4689f43206178ff5b899c71e96ed8 Mon Sep 17 00:00:00 2001 From: Graeme Yeates Date: Sat, 15 Oct 2016 12:05:09 -0400 Subject: [PATCH 1/2] Short circuit loops completely for some,every,detect when ready [fixes #1293] --- lib/internal/breakLoop.js | 3 +++ lib/internal/createTester.js | 24 ++++++++++-------------- lib/internal/eachOfLimit.js | 7 +++++-- mocha_test/detect.js | 26 ++++++++++++++++++++++++++ mocha_test/some.js | 29 ++++++++++++++++++++++++++++- 5 files changed, 72 insertions(+), 17 deletions(-) create mode 100644 lib/internal/breakLoop.js diff --git a/lib/internal/breakLoop.js b/lib/internal/breakLoop.js new file mode 100644 index 000000000..6bb216e8c --- /dev/null +++ b/lib/internal/breakLoop.js @@ -0,0 +1,3 @@ +// A temporary value used to identify if the loop should be broken. +// See #1064, #1293 +export default {}; \ No newline at end of file diff --git a/lib/internal/createTester.js b/lib/internal/createTester.js index ac38bf75b..36223e619 100644 --- a/lib/internal/createTester.js +++ b/lib/internal/createTester.js @@ -1,28 +1,24 @@ 'use strict'; import noop from 'lodash/noop'; +import breakLoop from './breakLoop'; export default function _createTester(eachfn, check, getResult) { return function(arr, limit, iteratee, cb) { - function done(err) { + function done() { if (cb) { - if (err) { - cb(err); - } else { - cb(null, getResult(false)); - } + cb(null, getResult(false)); } } function wrappedIteratee(x, _, callback) { if (!cb) return callback(); iteratee(x, function (err, v) { - if (cb) { - if (err) { - cb(err); - cb = iteratee = false; - } else if (check(v)) { - cb(null, getResult(true, x)); - cb = iteratee = false; - } + // Check cb as another iteratee may have resolved with a + // value or error since we started this iteratee + if (cb && (err || check(v))) { + if (err) cb(err); + else cb(err, getResult(true, x)); + cb = iteratee = false; + return callback(err, breakLoop); } callback(); }); diff --git a/lib/internal/eachOfLimit.js b/lib/internal/eachOfLimit.js index 61fd6a4b0..da46fa45c 100644 --- a/lib/internal/eachOfLimit.js +++ b/lib/internal/eachOfLimit.js @@ -4,6 +4,8 @@ import once from './once'; import iterator from './iterator'; import onlyOnce from './onlyOnce'; +import breakLoop from './breakLoop'; + export default function _eachOfLimit(limit) { return function (obj, iteratee, callback) { callback = once(callback || noop); @@ -14,13 +16,14 @@ export default function _eachOfLimit(limit) { var done = false; var running = 0; - function iterateeCallback(err) { + function iterateeCallback(err, value) { running -= 1; if (err) { done = true; callback(err); } - else if (done && running <= 0) { + else if (value === breakLoop || (done && running <= 0)) { + done = true; return callback(null); } else { diff --git a/mocha_test/detect.js b/mocha_test/detect.js index f30848520..b1ebf249a 100644 --- a/mocha_test/detect.js +++ b/mocha_test/detect.js @@ -1,5 +1,6 @@ var async = require('../lib'); var expect = require('chai').expect; +var _ = require('lodash'); describe("detect", function () { @@ -134,6 +135,31 @@ describe("detect", function () { }); }); + it('detectSeries doesn\'t cause stack overflow (#1293)', function(done) { + var arr = _.range(10000); + let calls = 0; + async.detectSeries(arr, function(data, cb) { + calls += 1; + async.setImmediate(_.partial(cb, null, true)); + }, function(err) { + expect(err).to.equal(null); + expect(calls).to.equal(1); + done(); + }); + }); + + it('detectLimit doesn\'t cause stack overflow (#1293)', function(done) { + var arr = _.range(10000); + let calls = 0; + async.detectLimit(arr, 100, function(data, cb) { + calls += 1; + async.setImmediate(_.partial(cb, null, true)); + }, function(err) { + expect(err).to.equal(null); + expect(calls).to.equal(100); + done(); + }); + }); it('find alias', function(){ expect(async.find).to.equal(async.detect); diff --git a/mocha_test/some.js b/mocha_test/some.js index 0a5a8da44..989fecb6a 100644 --- a/mocha_test/some.js +++ b/mocha_test/some.js @@ -1,5 +1,6 @@ var async = require('../lib'); var expect = require('chai').expect; +var _ = require('lodash'); describe("some", function () { @@ -83,7 +84,6 @@ describe("some", function () { }); }); - it('someLimit short-circuit', function(done){ var calls = 0; async.someLimit([3,1,2], 1, function(x, callback){ @@ -97,6 +97,33 @@ describe("some", function () { }); }); + + it('someSeries doesn\'t cause stack overflow (#1293)', function(done) { + var arr = _.range(10000); + let calls = 0; + async.someSeries(arr, function(data, cb) { + calls += 1; + async.setImmediate(_.partial(cb, null, true)); + }, function(err) { + expect(err).to.equal(null); + expect(calls).to.equal(1); + done(); + }); + }); + + it('someLimit doesn\'t cause stack overflow (#1293)', function(done) { + var arr = _.range(10000); + let calls = 0; + async.someLimit(arr, 100, function(data, cb) { + calls += 1; + async.setImmediate(_.partial(cb, null, true)); + }, function(err) { + expect(err).to.equal(null); + expect(calls).to.equal(100); + done(); + }); + }); + it('any alias', function(){ expect(async.any).to.equal(async.some); }); From 74c01fe6d46d978eeabc64716634dee1beb77c68 Mon Sep 17 00:00:00 2001 From: Graeme Yeates Date: Sat, 15 Oct 2016 12:12:20 -0400 Subject: [PATCH 2/2] Add short circuit test for every --- lib/internal/createTester.js | 5 +++-- mocha_test/every.js | 27 +++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/lib/internal/createTester.js b/lib/internal/createTester.js index 36223e619..b37d0f1dd 100644 --- a/lib/internal/createTester.js +++ b/lib/internal/createTester.js @@ -18,9 +18,10 @@ export default function _createTester(eachfn, check, getResult) { if (err) cb(err); else cb(err, getResult(true, x)); cb = iteratee = false; - return callback(err, breakLoop); + callback(err, breakLoop); + } else { + callback(); } - callback(); }); } if (arguments.length > 3) { diff --git a/mocha_test/every.js b/mocha_test/every.js index ed4693ebe..39ae51c4d 100644 --- a/mocha_test/every.js +++ b/mocha_test/every.js @@ -1,5 +1,6 @@ var async = require('../lib'); var expect = require('chai').expect; +var _ = require('lodash'); describe("every", function () { @@ -83,6 +84,32 @@ describe("every", function () { }); }); + it('everySeries doesn\'t cause stack overflow (#1293)', function(done) { + var arr = _.range(10000); + let calls = 0; + async.everySeries(arr, function(data, cb) { + calls += 1; + async.setImmediate(_.partial(cb, null, false)); + }, function(err) { + expect(err).to.equal(null); + expect(calls).to.equal(1); + done(); + }); + }); + + it('everyLimit doesn\'t cause stack overflow (#1293)', function(done) { + var arr = _.range(10000); + let calls = 0; + async.everyLimit(arr, 100, function(data, cb) { + calls += 1; + async.setImmediate(_.partial(cb, null, false)); + }, function(err) { + expect(err).to.equal(null); + expect(calls).to.equal(100); + done(); + }); + }); + it('all alias', function(){ expect(async.all).to.equal(async.every); });