Skip to content

Commit

Permalink
Fix #4 by setting no timeout for values above 2147483647 (max 32-bit …
Browse files Browse the repository at this point in the history
…integer value).
  • Loading branch information
MathieuTurcotte committed Mar 21, 2014
1 parent 3b14263 commit c09c100
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 1 deletion.
10 changes: 9 additions & 1 deletion lib/multiple.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ function WaitForMultiple(opts) {

this.eventName = options.event || WaitForMultiple.EVENT_NAME;

// TODO: Formalize the handling of 0 timeouts. Currently, a 0 timeout
// means no timeout at all which is probably unexpected.
this.timeoutDelay = options.timeout;
this.timeoutID = -1;

Expand All @@ -28,6 +30,12 @@ function WaitForMultiple(opts) {
}
util.inherits(WaitForMultiple, events.EventEmitter);

/**
* Timeout values too big to fit into a signed 32-bit integer will
* overflow resulting in the timeout being scheduled immediately.
*/
WaitForMultiple.MAX_TIMEOUT = 2147483647;

WaitForMultiple.EVENT_NAME = 'done';

WaitForMultiple.prototype.add = function(arg) {
Expand All @@ -41,7 +49,7 @@ WaitForMultiple.prototype.add = function(arg) {
WaitForMultiple.prototype.wait = function() {
this.startListening();

if (this.timeoutDelay) {
if (this.timeoutDelay && this.timeoutDelay <= WaitForMultiple.MAX_TIMEOUT) {
this.timeoutID = setTimeout(this.handlers.timeout, this.timeoutDelay);
}
};
Expand Down
17 changes: 17 additions & 0 deletions tests/multiple.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,23 @@ exports["WaitForMultiple"] = {
test.done();
},

"timeouts above MAX_TIMEOUT should result in no timeout": function(test) {
var hasTimedOut = false;

var waiter = new WaitForNothing({
timeout: WaitForMultiple.MAX_TIMEOUT + 1
});
waiter.once('timeout', function() {
hasTimedOut = true;
});
waiter.wait();

this.clock.tick(WaitForMultiple.MAX_TIMEOUT + 2);
test.ok(!hasTimedOut);

test.done();
},

"listeners should be removed on 'timeout'": function(test) {
var e1 = new events.EventEmitter();
var e2 = new events.EventEmitter();
Expand Down

0 comments on commit c09c100

Please sign in to comment.