timers: add internal [@@ refresh()] function

Hidden via a symbol because I'm unsure exactly what the API should look like in the end. Removes the need to use _unrefActive for efficiently refreshing timeouts. It still uses it under the hood but that could be replaced with insert() directly if it were in the same file. PR-URL: nodejs#18065 Reviewed-By: Anatoli Papirovski <[email protected]>
MayaLekova · May 8, 2018 · 0164140 · 0164140
1 parent c5b13db
commit 0164140
Show file tree

Hide file tree

Showing 5 changed files with 90 additions and 13 deletions.
diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js
@@ -57,11 +57,10 @@ const {
 const {
   kTimeout,
   setUnrefTimeout,
-  validateTimerDuration
+  validateTimerDuration,
+  refreshFnSymbol
 } = require('internal/timers');
 
-const { _unrefActive } = require('timers');
-
 const { ShutdownWrap, WriteWrap } = process.binding('stream_wrap');
 const { constants } = binding;
 
@@ -912,7 +911,7 @@ class Http2Session extends EventEmitter {
   [kUpdateTimer]() {
     if (this.destroyed)
       return;
-    if (this[kTimeout]) _unrefActive(this[kTimeout]);
+    if (this[kTimeout]) this[kTimeout][refreshFnSymbol]();
   }
 
   // Sets the id of the next stream to be created by this Http2Session.
@@ -1478,7 +1477,7 @@ class Http2Stream extends Duplex {
     if (this.destroyed)
       return;
     if (this[kTimeout])
-      _unrefActive(this[kTimeout]);
+      this[kTimeout][refreshFnSymbol]();
     if (this[kSession])
       this[kSession][kUpdateTimer]();
   }

diff --git a/lib/internal/timers.js b/lib/internal/timers.js
@@ -19,12 +19,16 @@ const errors = require('internal/errors');
 // Timeout values > TIMEOUT_MAX are set to 1.
 const TIMEOUT_MAX = 2 ** 31 - 1;
 
+const refreshFnSymbol = Symbol('refresh()');
+const unrefedSymbol = Symbol('unrefed');
+
 module.exports = {
   TIMEOUT_MAX,
   kTimeout: Symbol('timeout'), // For hiding Timeouts on other internals.
   async_id_symbol,
   trigger_async_id_symbol,
   Timeout,
+  refreshFnSymbol,
   setUnrefTimeout,
   validateTimerDuration
 };
@@ -39,7 +43,7 @@ function getTimers() {
 
 // Timer constructor function.
 // The entire prototype is defined in lib/timers.js
-function Timeout(callback, after, args, isRepeat) {
+function Timeout(callback, after, args, isRepeat, isUnrefed) {
   after *= 1; // coalesce to number or NaN
   if (!(after >= 1 && after <= TIMEOUT_MAX)) {
     if (after > TIMEOUT_MAX) {
@@ -64,6 +68,8 @@ function Timeout(callback, after, args, isRepeat) {
   this._repeat = isRepeat ? after : null;
   this._destroyed = false;
 
+  this[unrefedSymbol] = isUnrefed;
+
   this[async_id_symbol] = ++async_id_fields[kAsyncIdCounter];
   this[trigger_async_id_symbol] = getDefaultTriggerAsyncId();
   if (async_hook_fields[kInit] > 0) {
@@ -74,6 +80,19 @@ function Timeout(callback, after, args, isRepeat) {
   }
 }
 
+Timeout.prototype[refreshFnSymbol] = function refresh() {
+  if (this._handle) {
+    // Would be more ideal with uv_timer_again(), however that API does not
+    // cause libuv's sorted timers data structure (a binary heap at the time
+    // of writing) to re-sort itself. This causes ordering inconsistencies.
+    this._handle.stop();
+    this._handle.start(this._idleTimeout);
+  } else if (this[unrefedSymbol]) {
+    getTimers()._unrefActive(this);
+  } else {
+    getTimers().active(this);
+  }
+};
 
 function setUnrefTimeout(callback, after, arg1, arg2, arg3) {
   // Type checking identical to setTimeout()
@@ -102,7 +121,7 @@ function setUnrefTimeout(callback, after, arg1, arg2, arg3) {
       break;
   }
 
-  const timer = new Timeout(callback, after, args, false);
+  const timer = new Timeout(callback, after, args, false, true);
   getTimers()._unrefActive(timer);
 
   return timer;

diff --git a/lib/net.js b/lib/net.js
@@ -23,7 +23,6 @@
 
 const EventEmitter = require('events');
 const stream = require('stream');
-const timers = require('timers');
 const util = require('util');
 const internalUtil = require('internal/util');
 const {
@@ -64,7 +63,8 @@ const exceptionWithHostPort = util._exceptionWithHostPort;
 const {
   kTimeout,
   setUnrefTimeout,
-  validateTimerDuration
+  validateTimerDuration,
+  refreshFnSymbol
 } = require('internal/timers');
 
 function noop() {}
@@ -291,7 +291,7 @@ util.inherits(Socket, stream.Duplex);
 Socket.prototype._unrefTimer = function _unrefTimer() {
   for (var s = this; s !== null; s = s._parent) {
     if (s[kTimeout])
-      timers._unrefActive(s[kTimeout]);
+      s[kTimeout][refreshFnSymbol]();
   }
 };
 

diff --git a/lib/timers.js b/lib/timers.js
@@ -432,15 +432,15 @@ function setTimeout(callback, after, arg1, arg2, arg3) {
       break;
   }
 
-  const timeout = new Timeout(callback, after, args, false);
+  const timeout = new Timeout(callback, after, args, false, false);
   active(timeout);
 
   return timeout;
 }
 
 setTimeout[internalUtil.promisify.custom] = function(after, value) {
   const promise = createPromise();
-  const timeout = new Timeout(promise, after, [value], false);
+  const timeout = new Timeout(promise, after, [value], false, false);
   active(timeout);
 
   return promise;
@@ -523,7 +523,7 @@ exports.setInterval = function(callback, repeat, arg1, arg2, arg3) {
       break;
   }
 
-  const timeout = new Timeout(callback, repeat, args, true);
+  const timeout = new Timeout(callback, repeat, args, true, false);
   active(timeout);
 
   return timeout;

diff --git a/test/parallel/test-timers-refresh.js b/test/parallel/test-timers-refresh.js
@@ -0,0 +1,59 @@
+// Flags: --expose-internals
+
+'use strict';
+
+const common = require('../common');
+
+const { strictEqual } = require('assert');
+const { setUnrefTimeout, refreshFnSymbol } = require('internal/timers');
+
+// Schedule the unrefed cases first so that the later case keeps the event loop
+// active.
+
+// Every case in this test relies on implicit sorting within either Node's or
+// libuv's timers storage data structures.
+
+// unref()'d timer
+{
+  let called = false;
+  const timer = setTimeout(common.mustCall(() => {
+    called = true;
+  }), 1);
+  timer.unref();
+
+  // This relies on implicit timers handle sorting withing libuv.
+
+  setTimeout(common.mustCall(() => {
+    strictEqual(called, false, 'unref()\'d timer returned before check');
+  }), 1);
+
+  timer[refreshFnSymbol]();
+}
+
+// unref pooled timer
+{
+  let called = false;
+  const timer = setUnrefTimeout(common.mustCall(() => {
+    called = true;
+  }), 1);
+
+  setUnrefTimeout(common.mustCall(() => {
+    strictEqual(called, false, 'unref pooled timer returned before check');
+  }), 1);
+
+  timer[refreshFnSymbol]();
+}
+
+// regular timer
+{
+  let called = false;
+  const timer = setTimeout(common.mustCall(() => {
+    called = true;
+  }), 1);
+
+  setTimeout(common.mustCall(() => {
+    strictEqual(called, false, 'pooled timer returned before check');
+  }), 1);
+
+  timer[refreshFnSymbol]();
+}