Skip to content

Commit

Permalink
lib: fix guard expression in timer.unref()
Browse files Browse the repository at this point in the history
Fixes the following assertion on slow systems, like our ARM buildbot:

    $ out/Debug/node test/simple/test-timers-unref.js
    node: ../src/async-wrap-inl.h:101: v8::Handle<v8::Value>
    node::AsyncWrap::MakeCallback(uint32_t, int,
    v8::Handle<v8::Value>*): Assertion `cb_v->IsFunction()' failed.
    Aborted

The reason it only manifests on slow systems is that the test starts
a 1 ms interval timer, then defers timer.unref.bind({}) to the next
tick.  On fast systems, the test completes in under a millisecond,
before the callback is called.

This commit makes timer.unref() check that the receiver actually has
a timeout callback property.

Fixes nodejs#13.

PR-URL: nodejs/node#165
Reviewed-By: Rod Vagg <rod@vagg.org>
  • Loading branch information
bnoordhuis committed Dec 18, 2014
1 parent b7d2478 commit ebf9f29
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 3 deletions.
6 changes: 3 additions & 3 deletions lib/timers.js
Expand Up @@ -287,7 +287,9 @@ var Timeout = function(after) {
};

Timeout.prototype.unref = function() {
if (!this._handle) {
if (this._handle) {
this._handle.unref();
} else if (typeof(this._onTimeout) === 'function') {
var now = Timer.now();
if (!this._idleStart) this._idleStart = now;
var delay = this._idleStart + this._idleTimeout - now;
Expand All @@ -298,8 +300,6 @@ Timeout.prototype.unref = function() {
this._handle.start(delay, 0);
this._handle.domain = this.domain;
this._handle.unref();
} else {
this._handle.unref();
}
};

Expand Down
25 changes: 25 additions & 0 deletions test/parallel/test-timers-unref-call.js
@@ -0,0 +1,25 @@
// Copyright (c) 2014, StrongLoop Inc.
//
// Permission to use, copy, modify, and/or distribute this software for any
// purpose with or without fee is hereby granted, provided that the above
// copyright notice and this permission notice appear in all copies.
//
// THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
// WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
// MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
// ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
// WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
// ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
// OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.

var common = require('../common');

var Timer = process.binding('timer_wrap').Timer;
Timer.now = function() { return ++Timer.now.ticks; };
Timer.now.ticks = 0;

var t = setInterval(function() {}, 1);
var o = { _idleStart: 0, _idleTimeout: 1 };
t.unref.call(o);

setTimeout(clearInterval.bind(null, t), 2);

0 comments on commit ebf9f29

Please sign in to comment.