Skip to content
This repository has been archived by the owner on May 4, 2018. It is now read-only.

Commit

Permalink
unix: don't rely on libev to track timer state
Browse files Browse the repository at this point in the history
An obscure libev bug sometimes makes it miss state changes. Keep track of the
state ourselves.

Fixes nodejs/node-v0.x-archive#2515.
  • Loading branch information
bnoordhuis committed Feb 28, 2012
1 parent ec0eff9 commit 2f886c8
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 11 deletions.
46 changes: 36 additions & 10 deletions src/unix/core.c
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ static void uv__finish_close(uv_handle_t* handle);

void uv_close(uv_handle_t* handle, uv_close_cb close_cb) {
uv_async_t* async;
uv_timer_t* timer;
uv_stream_t* stream;
uv_process_t* process;

Expand Down Expand Up @@ -118,11 +117,7 @@ void uv_close(uv_handle_t* handle, uv_close_cb close_cb) {
break;

case UV_TIMER:
timer = (uv_timer_t*)handle;
if (ev_is_active(&timer->timer_watcher)) {
ev_ref(timer->loop->ev);
}
ev_timer_stop(timer->loop->ev, &timer->timer_watcher);
uv_timer_stop((uv_timer_t*)handle);
break;

case UV_PROCESS:
Expand Down Expand Up @@ -531,10 +526,23 @@ int uv_async_send(uv_async_t* async) {
}


static int uv__timer_active(const uv_timer_t* timer) {
return timer->flags & UV_TIMER_ACTIVE;
}


static int uv__timer_repeating(const uv_timer_t* timer) {
return timer->flags & UV_TIMER_REPEAT;
}


static void uv__timer_cb(EV_P_ ev_timer* w, int revents) {
uv_timer_t* timer = container_of(w, uv_timer_t, timer_watcher);

if (!ev_is_active(w)) {
assert(uv__timer_active(timer));

if (!uv__timer_repeating(timer)) {
timer->flags &= ~UV_TIMER_ACTIVE;
ev_ref(EV_A);
}

Expand All @@ -556,43 +564,61 @@ int uv_timer_init(uv_loop_t* loop, uv_timer_t* timer) {

int uv_timer_start(uv_timer_t* timer, uv_timer_cb cb, int64_t timeout,
int64_t repeat) {
if (ev_is_active(&timer->timer_watcher)) {
if (uv__timer_active(timer)) {
return -1;
}

timer->timer_cb = cb;
timer->flags |= UV_TIMER_ACTIVE;

if (repeat)
timer->flags |= UV_TIMER_REPEAT;
else
timer->flags &= ~UV_TIMER_REPEAT;

ev_timer_set(&timer->timer_watcher, timeout / 1000.0, repeat / 1000.0);
ev_timer_start(timer->loop->ev, &timer->timer_watcher);
ev_unref(timer->loop->ev);

return 0;
}


int uv_timer_stop(uv_timer_t* timer) {
if (ev_is_active(&timer->timer_watcher)) {
if (uv__timer_active(timer)) {
ev_ref(timer->loop->ev);
}

timer->flags &= ~(UV_TIMER_ACTIVE | UV_TIMER_REPEAT);
ev_timer_stop(timer->loop->ev, &timer->timer_watcher);

return 0;
}


int uv_timer_again(uv_timer_t* timer) {
if (!ev_is_active(&timer->timer_watcher)) {
if (!uv__timer_active(timer)) {
uv__set_sys_error(timer->loop, EINVAL);
return -1;
}

assert(uv__timer_repeating(timer));
ev_timer_again(timer->loop->ev, &timer->timer_watcher);
return 0;
}


void uv_timer_set_repeat(uv_timer_t* timer, int64_t repeat) {
assert(timer->type == UV_TIMER);
timer->timer_watcher.repeat = repeat / 1000.0;

if (repeat)
timer->flags |= UV_TIMER_REPEAT;
else
timer->flags &= ~UV_TIMER_REPEAT;
}


int64_t uv_timer_get_repeat(uv_timer_t* timer) {
assert(timer->type == UV_TIMER);
return (int64_t)(1000 * timer->timer_watcher.repeat);
Expand Down
1 change: 1 addition & 0 deletions src/unix/ev/ev.c
Original file line number Diff line number Diff line change
Expand Up @@ -2554,6 +2554,7 @@ void
ev_unref (EV_P)
{
--activecnt;
if (activecnt < 0) abort();
}

void
Expand Down
4 changes: 3 additions & 1 deletion src/unix/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,9 @@ enum {
UV_READABLE = 0x20, /* The stream is readable */
UV_WRITABLE = 0x40, /* The stream is writable */
UV_TCP_NODELAY = 0x080, /* Disable Nagle. */
UV_TCP_KEEPALIVE = 0x100 /* Turn on keep-alive. */
UV_TCP_KEEPALIVE = 0x100, /* Turn on keep-alive. */
UV_TIMER_ACTIVE = 0x080,
UV_TIMER_REPEAT = 0x100
};

/* core */
Expand Down

3 comments on commit 2f886c8

@kapouer
Copy link

@kapouer kapouer commented on 2f886c8 Mar 5, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i have hard time finding a reference to said libev bug... Did you take it to tje libev mailing list or somewhere else ?

@bnoordhuis
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, but I noticed that the libev guy committed a fix for this issue a couple of days after the initial version of my patch.

@kapouer
Copy link

@kapouer kapouer commented on 2f886c8 Mar 5, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it should be included in libev-4.11, if that is the patch you're referring to :
http://cvs.schmorp.de/libev/ev.c?r1=1.405&r2=1.407

Please sign in to comment.