From d22b2a934a62087522511e1e6b66b71370506c77 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Fri, 3 Apr 2015 01:14:08 +0300 Subject: [PATCH] timers: do not restart the interval after close Partially revert 776b73b24306bac0ce299df4f90b7645d5efca31. Following code crashes after backported timer leak fixes: ```javascript var timer = setInterval(function() { clearInterval(timer); }, 10); timer.unref(); ``` Note that this is actually tested in a `test-timers-unref.js`, and is crashing only with 776b73b24306bac0ce299df4f90b7645d5efca31. Calling `clearInterval` leads to the crashes in case of `.unref()`ed timers, and might lead to a extra timer spin in case of regular intervals that was closed during the interval callback. All of these happens because `.unref()`ed timer has it's own `_handle` and was used after the `.close()`. PR-URL: https://github.com/iojs/io.js/pull/1330 Reviewed-by: Trevor Norris --- lib/timers.js | 5 +++++ src/timer_wrap.cc | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/lib/timers.js b/lib/timers.js index 442789439bc7c2..dea10136370a09 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -272,6 +272,11 @@ exports.setInterval = function(callback, repeat) { function wrapper() { timer._repeat.call(this); + + // Timer might be closed - no point in restarting it + if (!timer._repeat) + return; + // If timer is unref'd (or was - it's permanently removed from the list.) if (this._handle) { this._handle.start(repeat, 0); diff --git a/src/timer_wrap.cc b/src/timer_wrap.cc index d71213f2202984..f65290a5162398 100644 --- a/src/timer_wrap.cc +++ b/src/timer_wrap.cc @@ -73,6 +73,8 @@ class TimerWrap : public HandleWrap { static void Start(const FunctionCallbackInfo& args) { TimerWrap* wrap = Unwrap(args.Holder()); + CHECK(HandleWrap::IsAlive(wrap)); + int64_t timeout = args[0]->IntegerValue(); int64_t repeat = args[1]->IntegerValue(); int err = uv_timer_start(&wrap->handle_, OnTimeout, timeout, repeat); @@ -82,6 +84,8 @@ class TimerWrap : public HandleWrap { static void Stop(const FunctionCallbackInfo& args) { TimerWrap* wrap = Unwrap(args.Holder()); + CHECK(HandleWrap::IsAlive(wrap)); + int err = uv_timer_stop(&wrap->handle_); args.GetReturnValue().Set(err); } @@ -89,6 +93,8 @@ class TimerWrap : public HandleWrap { static void Again(const FunctionCallbackInfo& args) { TimerWrap* wrap = Unwrap(args.Holder()); + CHECK(HandleWrap::IsAlive(wrap)); + int err = uv_timer_again(&wrap->handle_); args.GetReturnValue().Set(err); } @@ -96,6 +102,8 @@ class TimerWrap : public HandleWrap { static void SetRepeat(const FunctionCallbackInfo& args) { TimerWrap* wrap = Unwrap(args.Holder()); + CHECK(HandleWrap::IsAlive(wrap)); + int64_t repeat = args[0]->IntegerValue(); uv_timer_set_repeat(&wrap->handle_, repeat); args.GetReturnValue().Set(0); @@ -104,6 +112,8 @@ class TimerWrap : public HandleWrap { static void GetRepeat(const FunctionCallbackInfo& args) { TimerWrap* wrap = Unwrap(args.Holder()); + CHECK(HandleWrap::IsAlive(wrap)); + int64_t repeat = uv_timer_get_repeat(&wrap->handle_); if (repeat <= 0xfffffff) args.GetReturnValue().Set(static_cast(repeat));