timers: fix arbitrary object clearImmediate errors

Linkgoron · ruyadorno · commit d44b268910a7 · 2021-03-29T20:17:19.000-04:00
Fix errors that are caused by invoking clearImmediate with arbitrary objects. fixes: #37806 PR-URL: #37824 Fixes: #37806 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
diff --git 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
@@ -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
@@ -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
@@ -0,0 +1,8 @@
+'use strict';
+require('../common');
+
+// This test makes sure clearing timers with
+// objects doesn't throw
+clearImmediate({});
+clearTimeout({});
+clearInterval({});

Original file line number	Diff line number	Diff line change
`@@ -279,11 +279,11 @@ ImmediateList.prototype.append = function(item) {`
`279`	`279`	`// Removes an item from the linked list, adjusting the pointers of adjacent`
`280`	`280`	`// items and the linked list's head or tail pointers as necessary`
`281`	`281`	`ImmediateList.prototype.remove = function(item) {`
`282`		`- if (item._idleNext !== null) {`
	`282`	`+ if (item._idleNext) {`
`283`	`283`	`item._idleNext._idlePrev = item._idlePrev;`
`284`	`284`	`}`
`285`	`285`
`286`		`- if (item._idlePrev !== null) {`
	`286`	`+ if (item._idlePrev) {`
`287`	`287`	`item._idlePrev._idleNext = item._idleNext;`
`288`	`288`	`}`
`289`	`289`
Original file line number	Diff line number	Diff line change
`@@ -283,7 +283,7 @@ function clearImmediate(immediate) {`
`283`	`283`	`toggleImmediateRef(false);`
`284`	`284`	`immediate[kRefed] = null;`
`285`	`285`
`286`		`- if (destroyHooksExist()) {`
	`286`	`+ if (destroyHooksExist() && immediate[async_id_symbol] !== undefined) {`
`287`	`287`	`emitDestroy(immediate[async_id_symbol]);`
`288`	`288`	`}`
`289`	`289`