Skip to content

Commit

Permalink
timers: do not restart the interval after close
Browse files Browse the repository at this point in the history
Partially revert 776b73b.

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 776b73b.

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: #1330
Reviewed-by: Trevor Norris <[email protected]>
  • Loading branch information
indutny committed Apr 3, 2015
1 parent cca5efb commit d22b2a9
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 0 deletions.
5 changes: 5 additions & 0 deletions lib/timers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
10 changes: 10 additions & 0 deletions src/timer_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ class TimerWrap : public HandleWrap {
static void Start(const FunctionCallbackInfo<Value>& args) {
TimerWrap* wrap = Unwrap<TimerWrap>(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);
Expand All @@ -82,20 +84,26 @@ class TimerWrap : public HandleWrap {
static void Stop(const FunctionCallbackInfo<Value>& args) {
TimerWrap* wrap = Unwrap<TimerWrap>(args.Holder());

CHECK(HandleWrap::IsAlive(wrap));

int err = uv_timer_stop(&wrap->handle_);
args.GetReturnValue().Set(err);
}

static void Again(const FunctionCallbackInfo<Value>& args) {
TimerWrap* wrap = Unwrap<TimerWrap>(args.Holder());

CHECK(HandleWrap::IsAlive(wrap));

int err = uv_timer_again(&wrap->handle_);
args.GetReturnValue().Set(err);
}

static void SetRepeat(const FunctionCallbackInfo<Value>& args) {
TimerWrap* wrap = Unwrap<TimerWrap>(args.Holder());

CHECK(HandleWrap::IsAlive(wrap));

int64_t repeat = args[0]->IntegerValue();
uv_timer_set_repeat(&wrap->handle_, repeat);
args.GetReturnValue().Set(0);
Expand All @@ -104,6 +112,8 @@ class TimerWrap : public HandleWrap {
static void GetRepeat(const FunctionCallbackInfo<Value>& args) {
TimerWrap* wrap = Unwrap<TimerWrap>(args.Holder());

CHECK(HandleWrap::IsAlive(wrap));

int64_t repeat = uv_timer_get_repeat(&wrap->handle_);
if (repeat <= 0xfffffff)
args.GetReturnValue().Set(static_cast<uint32_t>(repeat));
Expand Down

0 comments on commit d22b2a9

Please sign in to comment.