unix: don't rely on libev to track timer state

An obscure libev bug sometimes makes it miss state changes. Keep track of the state ourselves. Fixes nodejs/node-v0.x-archive#2515.
joyent · Feb 28, 2012 · 2f886c8 · 2f886c8 · kapouer · Mar 5, 2012
1 parent ec0eff9
commit 2f886c8
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 11 deletions.
diff --git a/src/unix/core.c b/src/unix/core.c
@@ -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;
 
@@ -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:
@@ -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);
   }
 
@@ -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);

diff --git a/src/unix/ev/ev.c b/src/unix/ev/ev.c
@@ -2554,6 +2554,7 @@ void
 ev_unref (EV_P)
 {
   --activecnt;
+  if (activecnt < 0) abort();
 }
 
 void

diff --git a/src/unix/internal.h b/src/unix/internal.h
@@ -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 */