Merge pull request #1306 from caolan/break-tester-loops

Short circuit loops completely for some,every,detect when ready

6 files changed, 101 insertions, 18 deletions

diff --git a/lib/internal/breakLoop.js b/lib/internal/breakLoop.js
new file mode 100644
index 0000000..6bb216e
--- /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 ac38bf7..b37d0f1 100644
--- a/lib/internal/createTester.js
+++ b/lib/internal/createTester.js
@@ -1,30 +1,27 @@
 '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;
+                    callback(err, breakLoop);
+                } else {
+                    callback();
                 }
-                callback();
             });
         }
         if (arguments.length > 3) {
diff --git a/lib/internal/eachOfLimit.js b/lib/internal/eachOfLimit.js
index 61fd6a4..da46fa4 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 f308485..b1ebf24 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/every.js b/mocha_test/every.js
index ed4693e..39ae51c 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);
     });
diff --git a/mocha_test/some.js b/mocha_test/some.js
index 0a5a8da..989fecb 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);
     });