clean up parent records to remove memory leak, fixes #4

theKashey · May 2, 2020 · 26c5c48 · 26c5c48
1 parent e664207
commit 26c5c48
Show file tree

Hide file tree

Showing 2 changed files with 71 additions and 9 deletions.
diff --git a/src/index.js b/src/index.js
@@ -19,23 +19,56 @@ function assignParents(modules) {
   return result;
 }
 
+function removeModuleFromParent(parent, removedChild) {
+  if(parent && parent.children && removedChild) {
+    parent.children = parent.children.filter(child => child !== removedChild);
+  }
+}
+
 function burn(cache, wipeList, lookup, callback, removeFromCache = removeFromCache_nodejs) {
-// Secondary wave
+  const parentReference = new Map();
+
+  const remove = (moduleName) => {
+    const lookupcache = lookup[moduleName];
+    const module = cache[moduleName];
+    parentReference.delete(moduleName);
+    if (lookupcache) {
+      lookupcache.parents.forEach(parent => {
+        if (!parentReference.has(parent)) {
+          parentReference.set(parent, []);
+        }
+        // set a flag to remove this module from a parent record
+        parentReference.get(parent).push(module);
+        wipeList.push(parent);
+      });
+      delete lookup[moduleName];
+    }
+    removeFromCache(moduleName);
+  }
+
+  // primary wave
+  // execute what was set to be removed
+  let removeList = wipeList;
+  wipeList = [];
+  removeList.forEach(remove);
+
+  // Secondary wave
+  // remove parents of evicted modules while possible
   while (wipeList.length) {
-    var removeList = wipeList;
+    removeList = wipeList;
     wipeList = [];
 
     removeList.forEach(function (moduleName) {
       if (callback(moduleName)) {
-        if (lookup[moduleName]) {
-          wipeList.push.apply(wipeList, lookup[moduleName].parents);
-          delete lookup[moduleName];
-        }
-
-        removeFromCache(moduleName);
+        remove(moduleName);
       }
     });
   }
+
+  // post cleanup - remove references from parent
+  parentReference.forEach(
+    (removedChildren, parent) => removedChildren.forEach(child => removeModuleFromParent(cache[parent], child))
+  );
 }
 
 function purge(cache, wipeList, callback, removeFromCache, parents) {
@@ -87,7 +120,6 @@ function wipeMap(cache, callback, waveCallback, removeFromCache) {
   const simpleIndex = buildIndexForward(cache);
   const compositeIndex = [...new Set([...simpleIndex, ...Object.keys(parents)])];
   callback(compositeIndex, name => {
-    removeFromCache(name);
     wipeList.push(name);
   });
   return purge(cache, wipeList, waveCallback, undefined, parents);

diff --git a/test/moduleTest.js b/test/moduleTest.js
@@ -19,6 +19,19 @@ var stubs = {
   }
 };
 
+function collectModules() {
+  const modules = require.cache;
+  var result = {};
+  Object.keys(modules).forEach(function (moduleName) {
+    const parent = modules[moduleName];
+    const line = parent.children || [];
+    line.forEach(({id: childName}) => {
+      result[childName] = true;
+    });
+  });
+  return Object.keys(result);
+}
+
 describe('cache wipe', function () {
   describe('standart flow', function () {
     it('should cache modules', function () {
@@ -68,10 +81,18 @@ describe('cache wipe', function () {
       assert.equal(a.fn(), 1);
       assert.equal(a.c_fn(), 1);
 
+      // A and B are in the cache
+      assert.notEqual(collectModules().indexOf(require.resolve('./src/c.js')), -1);
+      assert.notEqual(collectModules().indexOf(require.resolve('./src/b.js')), -1);
+
       wipe(null, function (stub, fileName) {
         return fileName.indexOf('test/src/c') > 0
       });
 
+      // A and B are in no longer the cache
+      assert.equal(collectModules().indexOf(require.resolve('./src/c.js')), -1);
+      assert.equal(collectModules().indexOf(require.resolve('./src/b.js')), -1);
+
       var c = require('./src/c.js');
       assert.equal(c.fn(), 1);
 
@@ -87,12 +108,21 @@ describe('cache wipe', function () {
       assert.equal(a.fn(), 1);
       assert.equal(a.c_fn(), 1);
 
+      // A and B are in the cache
+      assert.notEqual(collectModules().indexOf(require.resolve('./src/c.js')), -1);
+      assert.notEqual(collectModules().indexOf(require.resolve('./src/b.js')), -1);
+
       wipe(null, function (stub, fileName) {
         return fileName.indexOf('test/src/c') > 0
       }, function () {
         return false;
       });
 
+      // C not in
+      assert.equal(collectModules().indexOf(require.resolve('./src/c.js')), -1);
+      // B is in
+      assert.notEqual(collectModules().indexOf(require.resolve('./src/b.js')), -1);
+
       var c = require('./src/c.js');
       assert.equal(c.fn(), 1);