timers: clean up, async integration for unref, simplify clearTimeout

lib/timers.js

@@ -260,7 +260,7 @@ TimerWrap.prototype[kOnTimeout] = function listOnTimeout(now) {
       continue;
     }
 
-    tryOnTimeout(timer, list);
+    tryOnTimeout(timer);
   }
 
   // If `L.peek(list)` returned nothing, the list was either empty or we have
@@ -289,15 +289,15 @@ TimerWrap.prototype[kOnTimeout] = function listOnTimeout(now) {
 
 // An optimization so that the try/finally only de-optimizes (since at least v8
 // 4.7) what is in this smaller function.
-function tryOnTimeout(timer, list) {
+function tryOnTimeout(timer, start) {
   timer._called = true;
   const timerAsyncId = (typeof timer[async_id_symbol] === 'number') ?
     timer[async_id_symbol] : null;
   var threw = true;
   if (timerAsyncId !== null)
     emitBefore(timerAsyncId, timer[trigger_async_id_symbol]);
   try {
-    ontimeout(timer);
+    ontimeout(timer, start);
     threw = false;
   } finally {
     if (timerAsyncId !== null) {
@@ -442,8 +442,7 @@ function ontimeout(timer, start) {
     rearm(timer, start);
 }
 
-
-function rearm(timer, start) {
+function rearm(timer, start = TimerWrap.now()) {
   // // Do not re-arm unenroll'd or closed timers.
   if (timer._idleTimeout === -1) return;
 
@@ -466,8 +465,8 @@ function rearm(timer, start) {
 
 
 const clearTimeout = exports.clearTimeout = function(timer) {
-  if (timer && (timer[kOnTimeout] || timer._onTimeout)) {
-    timer[kOnTimeout] = timer._onTimeout = null;
+  if (timer && timer._onTimeout) {
+    timer._onTimeout = null;
     if (timer instanceof Timeout) {
       timer.close(); // for after === 0
     } else {
@@ -521,7 +520,7 @@ function unrefdHandle(now) {
   try {
     // Don't attempt to call the callback if it is not a function.
     if (typeof this.owner._onTimeout === 'function') {
-      ontimeout(this.owner, now);
+      tryOnTimeout(this.owner, now);
     }
   } finally {
     // Make sure we clean up if the callback is no longer a function

test/async-hooks/test-timers.setTimeout.js

@@ -0,0 +1,61 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('assert');
+const tick = require('./tick');
+const initHooks = require('./init-hooks');
+const { checkInvocations } = require('./hook-checks');
+const TIMEOUT = common.platformTimeout(100);
+
+const hooks = initHooks();
+hooks.enable();
+
+// install first timeout
+setTimeout(common.mustCall(ontimeout), TIMEOUT);
+const as = hooks.activitiesOfTypes('Timeout');
+assert.strictEqual(as.length, 1);
+const t1 = as[0];
+assert.strictEqual(t1.type, 'Timeout');
+assert.strictEqual(typeof t1.uid, 'number');
+assert.strictEqual(typeof t1.triggerAsyncId, 'number');
+checkInvocations(t1, { init: 1 }, 't1: when first timer installed');
+
+let timer;
+let t2;
+function ontimeout() {
+  checkInvocations(t1, { init: 1, before: 1 }, 't1: when first timer fired');
+
+  setTimeout(onSecondTimeout, TIMEOUT).unref();
+  const as = hooks.activitiesOfTypes('Timeout');
+  t2 = as[1];
+  assert.strictEqual(as.length, 2);
+  checkInvocations(t1, { init: 1, before: 1 },
+                   't1: when second timer installed');
+  checkInvocations(t2, { init: 1 },
+                   't2: when second timer installed');
+
+  timer = setTimeout(common.mustNotCall(), 2 ** 31 - 1);
+}
+
+function onSecondTimeout() {
+  const as = hooks.activitiesOfTypes('Timeout');
+  assert.strictEqual(as.length, 3);
+  checkInvocations(t1, { init: 1, before: 1, after: 1 },
+                   't1: when second timer fired');
+  checkInvocations(t2, { init: 1, before: 1 },
+                   't2: when second timer fired');
+  clearTimeout(timer);
+  tick(2);
+}
+
+process.on('exit', onexit);
+
+function onexit() {
+  hooks.disable();
+  hooks.sanityCheck('Timeout');
+
+  checkInvocations(t1, { init: 1, before: 1, after: 1, destroy: 1 },
+                   't1: when process exits');
+  checkInvocations(t2, { init: 1, before: 1, after: 1, destroy: 1 },
+                   't2: when process exits');
+}

test/parallel/test-timers-timeout-to-interval.js

@@ -0,0 +1,12 @@
+'use strict';
+const common = require('../common');
+
+// This isn't officially supported but nonetheless is something that is
+// currently possible and as such it shouldn't cause the process to crash
+
+const t = setTimeout(common.mustCall(() => {
+  if (t._repeat) {
+    clearInterval(t);
+  }
+  t._repeat = true;
+}, 2), 1);