fix(backport): memory leaks due to timer references outliving the timers (#2773) (#2773)

johnp · web-flow · commit 66eb7f48a7bd · 2024-08-20T10:52:46.000+03:00
fix: memory leaks due to timer references outliving the timers (#2773) * fix: remove unused setInterval spy * fix: delete timer references after callback This way we don't hold any strong reference to potentially large setTimeout/setImmediate contexts any more. Since we use neither `clearTimeout`, nor `clearImmediate`, we can just ignore those cases and don't need to install spies to handle deliberate clears.
diff --git a/lib/common.js b/lib/common.js
@@ -593,31 +593,35 @@ function deepEqual(expected, actual) {
   return expected === actual
 }
 
-const timeouts = []
-const intervals = []
-const immediates = []
+const timeouts = new Set()
+const immediates = new Set()
 
 const wrapTimer =
   (timer, ids) =>
-  (...args) => {
-    const id = timer(...args)
-    ids.push(id)
+  (callback, ...timerArgs) => {
+    const cb = (...callbackArgs) => {
+      try {
+        // eslint-disable-next-line n/no-callback-literal
+        callback(...callbackArgs)
+      } finally {
+        ids.delete(id)
+      }
+    }
+    const id = timer(cb, ...timerArgs)
+    ids.add(id)
     return id
   }
 
 const setTimeout = wrapTimer(timers.setTimeout, timeouts)
-const setInterval = wrapTimer(timers.setInterval, intervals)
 const setImmediate = wrapTimer(timers.setImmediate, immediates)
 
 function clearTimer(clear, ids) {
-  while (ids.length) {
-    clear(ids.shift())
-  }
+  ids.forEach(clear)
+  ids.clear()
 }
 
 function removeAllTimers() {
   clearTimer(clearTimeout, timeouts)
-  clearTimer(clearInterval, intervals)
   clearTimer(clearImmediate, immediates)
 }
 
@@ -762,7 +766,6 @@ module.exports = {
   removeAllTimers,
   restoreOverriddenRequests,
   setImmediate,
-  setInterval,
   setTimeout,
   stringifyRequest,
 }
diff --git a/tests/test_common.js b/tests/test_common.js
@@ -530,17 +530,14 @@ describe('`dataEqual()`', () => {
 
 it('testing timers are deleted correctly', done => {
   const timeoutSpy = sinon.spy()
-  const intervalSpy = sinon.spy()
   const immediateSpy = sinon.spy()
 
   common.setTimeout(timeoutSpy, 0)
-  common.setInterval(intervalSpy, 0)
   common.setImmediate(immediateSpy)
   common.removeAllTimers()
 
   setImmediate(() => {
     expect(timeoutSpy).to.not.have.been.called()
-    expect(intervalSpy).to.not.have.been.called()
     expect(immediateSpy).to.not.have.been.called()
     done()
   })
