From 0f0326beb5a21bd088a1c5cba6352b2a3632d4cc Mon Sep 17 00:00:00 2001 From: jayther Date: Fri, 11 Apr 2014 02:51:16 -0700 Subject: [PATCH 1/2] Fixed bug with PromiseQueue instances sharing same queue. Fixed issue with Async.PromiseQueue instances sharing the same queue in the prototype. Fix is to define _queue and _curPromise in the constructor. Added to Async-test to test for this case. --- src/utils/Async.js | 2 ++ test/spec/Async-test.js | 43 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/src/utils/Async.js b/src/utils/Async.js index 2a95f791b65..871c911b0cc 100644 --- a/src/utils/Async.js +++ b/src/utils/Async.js @@ -424,6 +424,8 @@ define(function (require, exports, module) { * has finished. */ function PromiseQueue() { + this._queue = []; + this._curPromise = null; } /** diff --git a/test/spec/Async-test.js b/test/spec/Async-test.js index 1cbaf0b4e1e..e6cf88d3f53 100644 --- a/test/spec/Async-test.js +++ b/test/spec/Async-test.js @@ -499,6 +499,49 @@ define(function (require, exports, module) { expect(queue._queue.length).toBe(0); expect(queue._curPromise).toBe(null); }); + + it("should be able to run two queues simultaneously without clashing", function () { + var queue2 = new PromiseQueue(), + q1FnInfo1 = makeFn("one"), + q1FnInfo2 = makeFn("two"), + q2FnInfo3 = makeFn("three"), + q2FnInfo4 = makeFn("four"); + + //queue one + queue.add(q1FnInfo1.fn); + queue.add(q1FnInfo2.fn); + expect(calledFns.one).toBe(true); + expect(calledFns.two).toBeUndefined(); + //queue one should have one in _queue, + //queue two should have zero in _queue + expect(queue._queue.length).toBe(1); + expect(queue2._queue.length).toBe(0); + + //queue two + queue2.add(q2FnInfo3.fn); + queue2.add(q2FnInfo4.fn); + expect(calledFns.three).toBe(true); + expect(calledFns.four).toBeUndefined(); + //queue one and two should have one in _queue + expect(queue._queue.length).toBe(1); + expect(queue2._queue.length).toBe(1); + + q1FnInfo1.deferred.resolve(); + expect(calledFns.two).toBe(true); + expect(queue._queue.length).toBe(0); + + q1FnInfo2.deferred.resolve(); + expect(queue._queue.length).toBe(0); + expect(queue._curPromise).toBe(null); + + q2FnInfo3.deferred.resolve(); + expect(calledFns.three).toBe(true); + expect(queue2._queue.length).toBe(0); + + q2FnInfo4.deferred.resolve(); + expect(queue2._queue.length).toBe(0); + expect(queue2._curPromise).toBe(null); + }); }); }); }); \ No newline at end of file From f60c89becdd562223bc5ff1e524e13639864bef8 Mon Sep 17 00:00:00 2001 From: jayther Date: Fri, 11 Apr 2014 23:12:49 -0700 Subject: [PATCH 2/2] Applied nitpicks in PromiseQueue shared queue fix. Set PromiseQueue.prototype._queue to null to avoid confusion. Removed unneeded _curPromise initialization in constructor. Signed-off-by: jayther --- src/utils/Async.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/utils/Async.js b/src/utils/Async.js index 871c911b0cc..20a9fc73a91 100644 --- a/src/utils/Async.js +++ b/src/utils/Async.js @@ -425,7 +425,6 @@ define(function (require, exports, module) { */ function PromiseQueue() { this._queue = []; - this._curPromise = null; } /** @@ -434,7 +433,7 @@ define(function (require, exports, module) { * The queue of operations to execute sequentially. Note that even if this array is empty, there might * still be an operation we need to wait on; that operation's promise is stored in _curPromise. */ - PromiseQueue.prototype._queue = []; + PromiseQueue.prototype._queue = null; /** * @private