fix: fix OOM when collections leak multiple times (#90)

Fixes #89
nolanlawson · Jan 27, 2024 · f115a74 · f115a74
1 parent cfb7161
commit f115a74
Show file tree

Hide file tree

Showing 4 changed files with 77 additions and 4 deletions.
diff --git a/src/metrics/collections/collections.js b/src/metrics/collections/collections.js
@@ -2,7 +2,8 @@ import { sortBy } from '../../util.js'
 import { prettifyStacktrace } from '../../prettifyStacktrace.js'
 import { promisePool } from '../../promisePool.js'
 
-const PROMISE_POOL_SIZE = 100 // avoid OOMs when lots of collections are leaking
+const COLLECTIONS_PROMISE_POOL_SIZE = 100 // avoid OOMs when lots of collections are leaking
+const STACKTRACES_PROMISE_POOL_SIZE = 10 // avoid OOMs when collections are leaking in multiple places
 
 export async function startTrackingCollections (page) {
   // The basic idea for this comes from
@@ -317,12 +318,12 @@ export async function augmentLeakingCollectionsWithStacktraces (page, collection
     return result
   }
 
-  return (await promisePool(PROMISE_POOL_SIZE, collections.map(collection => async () => {
+  return (await promisePool(COLLECTIONS_PROMISE_POOL_SIZE, collections.map(collection => async () => {
     const res = { ...collection }
     if (collection.id in idsToStacktraces) {
       const stacktraces = idsToStacktraces[collection.id]
-      res.stacktraces = await Promise.all(stacktraces.map(async stacktrace => {
-        return getStacktraceWithOriginalAndPretty(stacktrace)
+      res.stacktraces = await promisePool(STACKTRACES_PROMISE_POOL_SIZE, stacktraces.map(stacktrace => async () => {
+        return (await getStacktraceWithOriginalAndPretty(stacktrace))
       }))
     }
     return res

diff --git a/test/spec/collections.test.mjs b/test/spec/collections.test.mjs
@@ -163,6 +163,55 @@ context        webpack://navigo/src/Q.ts:31:31
 next           webpack://navigo/src/Q.ts:34:10
 done           webpack://navigo/src/middlewares/callHandler.ts:9:2
 context        webpack://navigo/src/Q.ts:31:31
+next           webpack://navigo/src/Q.ts:34:10
+    `
+    const { pretty } = firstCollection.stacktraces[0]
+    expect(normalizeStackTrace(pretty)).to.deep.equal(normalizeStackTrace(expected))
+  })
+
+  it('collections with lots of leaks', async () => {
+    const results = await asyncIterableToArray(findLeaks('http://localhost:3000/test/www/collectionsWithLotsOfLeaks/', {
+      iterations: 3
+    }))
+
+    const result = results[0].result
+    expect(result.leaks.detected).to.equal(true)
+    expect(result.leaks.collections.length).to.equal(5)
+
+    const firstCollection = result.leaks.collections[0]
+    const {
+      delta,
+      deltaPerIteration,
+      preview,
+      sizeAfter,
+      sizeBefore,
+      type
+    } = firstCollection
+    expect({
+      delta,
+      deltaPerIteration,
+      preview,
+      sizeAfter,
+      sizeBefore,
+      type
+    }).to.deep.equal({
+      delta: 6000,
+      deltaPerIteration: 2000,
+      preview: '[0, ...]',
+      sizeAfter: 8000,
+      sizeBefore: 2000,
+      type: 'Array'
+    })
+
+    const expected = `
+aboutHook      http://localhost:3000/test/www/collectionsWithLotsOfLeaks/script.js:10:18
+               webpack://navigo/src/middlewares/checkForAfterHook.ts:10:52
+Array.forEach  <anonymous>
+forEach        webpack://navigo/src/middlewares/checkForAfterHook.ts:10:36
+context        webpack://navigo/src/Q.ts:31:31
+next           webpack://navigo/src/Q.ts:34:10
+done           webpack://navigo/src/middlewares/callHandler.ts:9:2
+context        webpack://navigo/src/Q.ts:31:31
 next           webpack://navigo/src/Q.ts:34:10
     `
     const { pretty } = firstCollection.stacktraces[0]

diff --git a/test/www/collectionsWithLotsOfLeaks/index.html b/test/www/collectionsWithLotsOfLeaks/index.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+  <meta charset="UTF-8">
+</head>
+<body>
+<script src="../../../node_modules/navigo/lib/navigo.js"></script>
+<script type="module" src="./script.js"></script>
+</body>
+</html>
diff --git a/test/www/collectionsWithLotsOfLeaks/script.js b/test/www/collectionsWithLotsOfLeaks/script.js
@@ -0,0 +1,13 @@
+import { makeRouter } from '../basicRouter.js'
+
+const router = makeRouter(['', 'about'])
+
+const collections = Array(5).fill().map(_ => [])
+
+router.addAfterHook('/about', function aboutHook () {
+  for (const collection of collections) {
+    for (let i = 0; i < 2000; i++) {
+      collection.push(0)
+    }
+  }
+})