From d44b268910a71c4666033cdf59175d88d74beeb5 Mon Sep 17 00:00:00 2001 From: Nitzan Uziely Date: Sat, 20 Mar 2021 01:50:31 +0200 Subject: [PATCH] timers: fix arbitrary object clearImmediate errors Fix errors that are caused by invoking clearImmediate with arbitrary objects. fixes: https://github.com/nodejs/node/issues/37806 PR-URL: https://github.com/nodejs/node/pull/37824 Fixes: https://github.com/nodejs/node/issues/37806 Reviewed-By: Rich Trott Reviewed-By: Ruben Bridgewater Reviewed-By: Antoine du Hamel Reviewed-By: Benjamin Gruenbaum --- lib/internal/timers.js | 4 ++-- lib/timers.js | 2 +- test/parallel/test-repl-clear-immediate-crash.js | 12 ++++++++++++ .../test-timers-clear-object-does-not-throw-error.js | 8 ++++++++ 4 files changed, 23 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-repl-clear-immediate-crash.js create mode 100644 test/parallel/test-timers-clear-object-does-not-throw-error.js diff --git a/lib/internal/timers.js b/lib/internal/timers.js index 050d356b202d70..9521c343038b4f 100644 --- a/lib/internal/timers.js +++ b/lib/internal/timers.js @@ -279,11 +279,11 @@ ImmediateList.prototype.append = function(item) { // Removes an item from the linked list, adjusting the pointers of adjacent // items and the linked list's head or tail pointers as necessary ImmediateList.prototype.remove = function(item) { - if (item._idleNext !== null) { + if (item._idleNext) { item._idleNext._idlePrev = item._idlePrev; } - if (item._idlePrev !== null) { + if (item._idlePrev) { item._idlePrev._idleNext = item._idleNext; } diff --git a/lib/timers.js b/lib/timers.js index 3cce2e37b007e8..73844fdbcc7613 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -283,7 +283,7 @@ function clearImmediate(immediate) { toggleImmediateRef(false); immediate[kRefed] = null; - if (destroyHooksExist()) { + if (destroyHooksExist() && immediate[async_id_symbol] !== undefined) { emitDestroy(immediate[async_id_symbol]); } diff --git a/test/parallel/test-repl-clear-immediate-crash.js b/test/parallel/test-repl-clear-immediate-crash.js new file mode 100644 index 00000000000000..ce8aaf48e7fa0e --- /dev/null +++ b/test/parallel/test-repl-clear-immediate-crash.js @@ -0,0 +1,12 @@ +'use strict'; +const common = require('../common'); +const child_process = require('child_process'); +const assert = require('assert'); + +// Regression test for https://github.com/nodejs/node/issues/37806: +const proc = child_process.spawn(process.execPath, ['-i']); +proc.on('error', common.mustNotCall()); +proc.on('exit', common.mustCall((code) => { + assert.strictEqual(code, 0); +})); +proc.stdin.write('clearImmediate({});\n.exit\n'); diff --git a/test/parallel/test-timers-clear-object-does-not-throw-error.js b/test/parallel/test-timers-clear-object-does-not-throw-error.js new file mode 100644 index 00000000000000..9752f53abd0755 --- /dev/null +++ b/test/parallel/test-timers-clear-object-does-not-throw-error.js @@ -0,0 +1,8 @@ +'use strict'; +require('../common'); + +// This test makes sure clearing timers with +// objects doesn't throw +clearImmediate({}); +clearTimeout({}); +clearInterval({});