From 4f2000f9fecbd7102403cc24b962f4e40994243a Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Fri, 13 Apr 2018 10:37:28 +0200 Subject: [PATCH] timers: call destroy on interval error When an interval callback throws an error, the destroy hook is never called due to a faulty if condition. PR-URL: https://github.com/nodejs/node/pull/20001 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Ruben Bridgewater Reviewed-By: Matteo Collina Reviewed-By: James M Snell Reviewed-By: Trivikram Kamat Reviewed-By: Jeremiah Senkpiel --- lib/timers.js | 2 +- test/async-hooks/test-timers.setInterval.js | 37 +++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 test/async-hooks/test-timers.setInterval.js diff --git a/lib/timers.js b/lib/timers.js index 7d3e7b50309..15700f5a121 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -292,7 +292,7 @@ function tryOnTimeout(timer, start) { if (timerAsyncId !== null) { if (!threw) emitAfter(timerAsyncId); - if (!timer._repeat && destroyHooksExist() && + if ((threw || !timer._repeat) && destroyHooksExist() && !timer._destroyed) { emitDestroy(timerAsyncId); timer._destroyed = true; diff --git a/test/async-hooks/test-timers.setInterval.js b/test/async-hooks/test-timers.setInterval.js new file mode 100644 index 00000000000..a3434e134fa --- /dev/null +++ b/test/async-hooks/test-timers.setInterval.js @@ -0,0 +1,37 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const initHooks = require('./init-hooks'); +const { checkInvocations } = require('./hook-checks'); +const TIMEOUT = common.platformTimeout(100); + +const hooks = initHooks(); +hooks.enable(); + +setInterval(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 timer installed'); + +function ontimeout() { + checkInvocations(t1, { init: 1, before: 1 }, 't1: when first timer fired'); + + throw new Error('setInterval Error'); +} + +process.once('uncaughtException', common.mustCall((err) => { + assert.strictEqual(err.message, 'setInterval Error'); +})); + +process.on('exit', () => { + hooks.disable(); + hooks.sanityCheck('Timeout'); + + checkInvocations(t1, { init: 1, before: 1, after: 1, destroy: 1 }, + 't1: when process exits'); +});