src: refactor timers to remove TimerWrap

Refactor Timers to behave more similarly to Immediates by having a single uv_timer_t handle which is stored on the Environment. No longer expose timers in a public binding and instead make it part of the internalBinding. PR-URL: #20894 Fixes: #10154 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Tiancheng "Timothy" Gu <[email protected]> Reviewed-By: Gus Caplan <[email protected]>
nodejs · Jun 25, 2018 · 2930bd1 · 2930bd1
1 parent 6f63f8d
commit 2930bd1
Show file tree

Hide file tree

Showing 36 changed files with 218 additions and 320 deletions.
diff --git a/doc/api/async_hooks.md b/doc/api/async_hooks.md
@@ -238,7 +238,7 @@ resource's constructor.
 ```text
 FSEVENTWRAP, FSREQWRAP, GETADDRINFOREQWRAP, GETNAMEINFOREQWRAP, HTTPPARSER,
 JSSTREAM, PIPECONNECTWRAP, PIPEWRAP, PROCESSWRAP, QUERYWRAP, SHUTDOWNWRAP,
-SIGNALWRAP, STATWATCHER, TCPCONNECTWRAP, TCPSERVER, TCPWRAP, TIMERWRAP, TTYWRAP,
+SIGNALWRAP, STATWATCHER, TCPCONNECTWRAP, TCPSERVER, TCPWRAP, TTYWRAP,
 UDPSENDWRAP, UDPWRAP, WRITEWRAP, ZLIB, SSLCONNECTION, PBKDF2REQUEST,
 RANDOMBYTESREQUEST, TLSWRAP, Timeout, Immediate, TickObject
 ```

diff --git a/lib/internal/timers.js b/lib/internal/timers.js
@@ -6,6 +6,7 @@ const {
   initHooksExist,
   emitInit
 } = require('internal/async_hooks');
+const { internalBinding } = require('internal/bootstrap/loaders');
 // Symbols for storing async id state.
 const async_id_symbol = Symbol('asyncId');
 const trigger_async_id_symbol = Symbol('triggerId');
@@ -30,7 +31,8 @@ module.exports = {
   kRefed,
   initAsyncResource,
   setUnrefTimeout,
-  validateTimerDuration
+  validateTimerDuration,
+  getLibuvNow: internalBinding('timers').getLibuvNow,
 };
 
 var timers;

diff --git a/lib/timers.js b/lib/timers.js
@@ -21,10 +21,15 @@
 
 'use strict';
 
+const { internalBinding } = require('internal/bootstrap/loaders');
 const {
-  Timer: TimerWrap,
+  getLibuvNow,
   setupTimers,
-} = process.binding('timer_wrap');
+  scheduleTimer,
+  toggleTimerRef,
+  immediateInfo,
+  toggleImmediateRef
+} = internalBinding('timers');
 const L = require('internal/linkedlist');
 const PriorityQueue = require('internal/priority_queue');
 const {
@@ -53,8 +58,9 @@ const kCount = 0;
 const kRefCount = 1;
 const kHasOutstanding = 2;
 
-const [immediateInfo, toggleImmediateRef] =
-  setupTimers(processImmediate, processTimers);
+// Call into C++ to assign callbacks that are responsible for processing
+// Immediates and TimerLists.
+setupTimers(processImmediate, processTimers);
 
 // HOW and WHY the timers implementation works the way it does.
 //
@@ -156,47 +162,38 @@ function setPosition(node, pos) {
   node.priorityQueuePosition = pos;
 }
 
-let handle = null;
 let nextExpiry = Infinity;
 
 let timerListId = Number.MIN_SAFE_INTEGER;
 let refCount = 0;
 
 function incRefCount() {
   if (refCount++ === 0)
-    handle.ref();
+    toggleTimerRef(true);
 }
 
 function decRefCount() {
   if (--refCount === 0)
-    handle.unref();
-}
-
-function createHandle(refed) {
-  debug('initial run, creating TimerWrap handle');
-  handle = new TimerWrap();
-  if (!refed)
-    handle.unref();
+    toggleTimerRef(false);
 }
 
 // Schedule or re-schedule a timer.
 // The item must have been enroll()'d first.
 const active = exports.active = function(item) {
-  insert(item, true, TimerWrap.now());
+  insert(item, true, getLibuvNow());
 };
 
 // Internal APIs that need timeouts should use `_unrefActive()` instead of
 // `active()` so that they do not unnecessarily keep the process open.
 exports._unrefActive = function(item) {
-  insert(item, false, TimerWrap.now());
+  insert(item, false, getLibuvNow());
 };
 
 
 // The underlying logic for scheduling or re-scheduling a timer.
 //
 // Appends a timer onto the end of an existing timers list, or creates a new
-// TimerWrap backed list if one does not already exist for the specified timeout
-// duration.
+// list if one does not already exist for the specified timeout duration.
 function insert(item, refed, start) {
   const msecs = item._idleTimeout;
   if (msecs < 0 || msecs === undefined)
@@ -213,9 +210,7 @@ function insert(item, refed, start) {
     queue.insert(list);
 
     if (nextExpiry > expiry) {
-      if (handle === null)
-        createHandle(refed);
-      handle.start(msecs);
+      scheduleTimer(msecs);
       nextExpiry = expiry;
     }
   }
@@ -252,32 +247,23 @@ function processTimers(now) {
 
   let list, ran;
   while (list = queue.peek()) {
-    if (list.expiry > now)
-      break;
+    if (list.expiry > now) {
+      nextExpiry = list.expiry;
+      return refCount > 0 ? nextExpiry : -nextExpiry;
+    }
     if (ran)
       runNextTicks();
+    else
+      ran = true;
     listOnTimeout(list, now);
-    ran = true;
   }
-
-  if (refCount > 0)
-    handle.ref();
-  else
-    handle.unref();
-
-  if (list !== undefined) {
-    nextExpiry = list.expiry;
-    handle.start(Math.max(nextExpiry - TimerWrap.now(), 1));
-  }
-
-  return true;
+  return 0;
 }
 
 function listOnTimeout(list, now) {
   const msecs = list.msecs;
 
   debug('timeout callback %d', msecs);
-  debug('now: %d', now);
 
   var diff, timer;
   while (timer = L.peek(list)) {
@@ -336,7 +322,7 @@ function listOnTimeout(list, now) {
 // 4.7) what is in this smaller function.
 function tryOnTimeout(timer, start) {
   if (start === undefined && timer._repeat)
-    start = TimerWrap.now();
+    start = getLibuvNow();
   try {
     ontimeout(timer);
   } finally {
@@ -474,7 +460,7 @@ function ontimeout(timer) {
 }
 
 function rearm(timer, start) {
-  // // Do not re-arm unenroll'd or closed timers.
+  // Do not re-arm unenroll'd or closed timers.
   if (timer._idleTimeout === -1)
     return;
 

diff --git a/node.gyp b/node.gyp
@@ -371,7 +371,7 @@
         'src/stream_pipe.cc',
         'src/stream_wrap.cc',
         'src/tcp_wrap.cc',
-        'src/timer_wrap.cc',
+        'src/timers.cc',
         'src/tracing/agent.cc',
         'src/tracing/node_trace_buffer.cc',
         'src/tracing/node_trace_writer.cc',

diff --git a/src/async_wrap.h b/src/async_wrap.h
@@ -63,7 +63,6 @@ namespace node {
   V(TCPCONNECTWRAP)                                                           \
   V(TCPSERVERWRAP)                                                            \
   V(TCPWRAP)                                                                  \
-  V(TIMERWRAP)                                                                \
   V(TTYWRAP)                                                                  \
   V(UDPSENDWRAP)                                                              \
   V(UDPWRAP)                                                                  \

diff --git a/src/env-inl.h b/src/env-inl.h
@@ -334,6 +334,14 @@ inline tracing::Agent* Environment::tracing_agent() const {
   return tracing_agent_;
 }
 
+inline Environment* Environment::from_timer_handle(uv_timer_t* handle) {
+  return ContainerOf(&Environment::timer_handle_, handle);
+}
+
+inline uv_timer_t* Environment::timer_handle() {
+  return &timer_handle_;
+}
+
 inline Environment* Environment::from_immediate_check_handle(
     uv_check_t* handle) {
   return ContainerOf(&Environment::immediate_check_handle_, handle);

diff --git a/src/env.cc b/src/env.cc
@@ -13,6 +13,7 @@
 namespace node {
 
 using v8::Context;
+using v8::Function;
 using v8::FunctionTemplate;
 using v8::HandleScope;
 using v8::Integer;
@@ -25,6 +26,7 @@ using v8::StackFrame;
 using v8::StackTrace;
 using v8::String;
 using v8::Symbol;
+using v8::TryCatch;
 using v8::Value;
 using worker::Worker;
 
@@ -173,6 +175,9 @@ void Environment::Start(int argc,
   HandleScope handle_scope(isolate());
   Context::Scope context_scope(context());
 
+  CHECK_EQ(0, uv_timer_init(event_loop(), timer_handle()));
+  uv_unref(reinterpret_cast<uv_handle_t*>(timer_handle()));
+
   uv_check_init(event_loop(), immediate_check_handle());
   uv_unref(reinterpret_cast<uv_handle_t*>(immediate_check_handle()));
 
@@ -227,6 +232,10 @@ void Environment::RegisterHandleCleanups() {
     env->CloseHandle(handle, [](uv_handle_t* handle) {});
   };
 
+  RegisterHandleCleanup(
+      reinterpret_cast<uv_handle_t*>(timer_handle()),
+      close_and_finish,
+      nullptr);
   RegisterHandleCleanup(
       reinterpret_cast<uv_handle_t*>(immediate_check_handle()),
       close_and_finish,
@@ -470,6 +479,78 @@ void Environment::RunAndClearNativeImmediates() {
 }
 
 
+void Environment::ScheduleTimer(int64_t duration_ms) {
+  uv_timer_start(timer_handle(), RunTimers, duration_ms, 0);
+}
+
+void Environment::ToggleTimerRef(bool ref) {
+  if (ref) {
+    uv_ref(reinterpret_cast<uv_handle_t*>(timer_handle()));
+  } else {
+    uv_unref(reinterpret_cast<uv_handle_t*>(timer_handle()));
+  }
+}
+
+void Environment::RunTimers(uv_timer_t* handle) {
+  Environment* env = Environment::from_timer_handle(handle);
+
+  if (!env->can_call_into_js())
+    return;
+
+  HandleScope handle_scope(env->isolate());
+  Context::Scope context_scope(env->context());
+
+  Local<Object> process = env->process_object();
+  InternalCallbackScope scope(env, process, {0, 0});
+
+  Local<Function> cb = env->timers_callback_function();
+  MaybeLocal<Value> ret;
+  Local<Value> arg = env->GetNow();
+  // This code will loop until all currently due timers will process. It is
+  // impossible for us to end up in an infinite loop due to how the JS-side
+  // is structured.
+  do {
+    TryCatch try_catch(env->isolate());
+    try_catch.SetVerbose(true);
+    ret = cb->Call(env->context(), process, 1, &arg);
+  } while (ret.IsEmpty() && env->can_call_into_js());
+
+  // NOTE(apapirovski): If it ever becomes possibble that `call_into_js` above
+  // is reset back to `true` after being previously set to `false` then this
+  // code becomes invalid and needs to be rewritten. Otherwise catastrophic
+  // timers corruption will occurr and all timers behaviour will become
+  // entirely unpredictable.
+  if (ret.IsEmpty())
+    return;
+
+  // To allow for less JS-C++ boundary crossing, the value returned from JS
+  // serves a few purposes:
+  // 1. If it's 0, no more timers exist and the handle should be unrefed
+  // 2. If it's > 0, the value represents the next timer's expiry and there
+  //    is at least one timer remaining that is refed.
+  // 3. If it's < 0, the absolute value represents the next timer's expiry
+  //    and there are no timers that are refed.
+  int64_t expiry_ms =
+      ret.ToLocalChecked()->IntegerValue(env->context()).FromJust();
+
+  uv_handle_t* h = reinterpret_cast<uv_handle_t*>(handle);
+
+  if (expiry_ms != 0) {
+    int64_t duration_ms =
+        llabs(expiry_ms) - (uv_now(env->event_loop()) - env->timer_base());
+
+    env->ScheduleTimer(duration_ms > 0 ? duration_ms : 1);
+
+    if (expiry_ms > 0)
+      uv_ref(h);
+    else
+      uv_unref(h);
+  } else {
+    uv_unref(h);
+  }
+}
+
+
 void Environment::CheckImmediate(uv_check_t* handle) {
   Environment* env = Environment::from_immediate_check_handle(handle);
 

diff --git a/src/env.h b/src/env.h
@@ -628,6 +628,9 @@ class Environment {
   inline uv_loop_t* event_loop() const;
   inline uint32_t watched_providers() const;
 
+  static inline Environment* from_timer_handle(uv_timer_t* handle);
+  inline uv_timer_t* timer_handle();
+
   static inline Environment* from_immediate_check_handle(uv_check_t* handle);
   inline uv_check_t* immediate_check_handle();
   inline uv_idle_t* immediate_idle_handle();
@@ -840,6 +843,8 @@ class Environment {
   static inline Environment* ForAsyncHooks(AsyncHooks* hooks);
 
   v8::Local<v8::Value> GetNow();
+  void ScheduleTimer(int64_t duration);
+  void ToggleTimerRef(bool ref);
 
   inline void AddCleanupHook(void (*fn)(void*), void* arg);
   inline void RemoveCleanupHook(void (*fn)(void*), void* arg);
@@ -857,6 +862,7 @@ class Environment {
   v8::Isolate* const isolate_;
   IsolateData* const isolate_data_;
   tracing::Agent* const tracing_agent_;
+  uv_timer_t timer_handle_;
   uv_check_t immediate_check_handle_;
   uv_idle_t immediate_idle_handle_;
   uv_prepare_t idle_prepare_handle_;
@@ -919,6 +925,8 @@ class Environment {
 
   worker::Worker* worker_context_ = nullptr;
 
+  static void RunTimers(uv_timer_t* handle);
+
   struct ExitCallback {
     void (*cb_)(void* arg);
     void* arg_;

diff --git a/src/node_internals.h b/src/node_internals.h
@@ -129,7 +129,7 @@ struct sockaddr;
     V(string_decoder)                                                         \
     V(symbols)                                                                \
     V(tcp_wrap)                                                               \
-    V(timer_wrap)                                                             \
+    V(timers)                                                             \
     V(trace_events)                                                           \
     V(tty_wrap)                                                               \
     V(types)                                                                  \