From ebf9f297b30d6cf2e5060da91d63cebbedc448e2 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Mon, 15 Dec 2014 17:25:31 +0100 Subject: [PATCH] lib: fix guard expression in timer.unref() 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 node::AsyncWrap::MakeCallback(uint32_t, int, v8::Handle*): 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 #13. PR-URL: https://github.com/iojs/io.js/pull/165 Reviewed-By: Rod Vagg --- lib/timers.js | 6 +++--- test/parallel/test-timers-unref-call.js | 25 +++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-timers-unref-call.js diff --git a/lib/timers.js b/lib/timers.js index 7fa683e..96d57ec 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -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; @@ -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(); } }; diff --git a/test/parallel/test-timers-unref-call.js b/test/parallel/test-timers-unref-call.js new file mode 100644 index 0000000..b6f7357 --- /dev/null +++ b/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); -- 2.7.4