Proxyquire is leaking on module.parent

[charlespwd]

This one took me a while to figure out, had to write my own "slim" proxyquire to find it.

Where I work, we use proxyquire to mock our modules and our test suite has around 1050 tests. We ran into issues where mocha --watch would crash after a couple of runs because the garbage collector was not able to garbage collect anymore.

If I compare successive heap dumps I notice that as time goes by I end up having more and more Module objects.

If you look at the contructor of Module, you'll notice that whenever we load a new module, this one gets pushed as a children of the module's parent.

In my slim proxyquire, the fix for the leak was to simply parent.children.pop() whenever I call Module._load and to also remove proxyquire from proxyquire's parent module.children.

[thlorenz]

Interesting. Never ran into this due to using/testing smaller modules, but I understand this can be annoying. As you probably understand we have to remove the proxyquire module from the cache in order to force re-require so we can grab the module that is requireing proxyquire. This is done here.

Do I understand correctly that simply removing it from the parent.children of the module as well would fix things? If so, please supply a PR and we'll review and verify that nothing else is affected by this.

If merged we should probably release a new major version (just to be sure as some people may depend on the current behavior).

[charlespwd]

Do I understand correctly that simply removing it from the parent.children of the module as well would fix things?

That could work. proxyquire itself would be leaking if it is called multiple times (e.g. in a beforeEach). But since the parent doesn't hold a reference to it in its children, it might be enough for the garbage collector.

I'd have to check.

[thlorenz]

proxyquire itself would be leaking

How would it be leaking? what's holding onto it?

[charlespwd]

That does not seem to be enough. Here's what my memory looks like after successive tests without a module.parent.children.pop() after every Module._load

ws/outbox/es5 (fix/mocha-memory-leaks ✗) $ ps -aux | awk -F' ' ' /bin\/_mocha/ { print $5,$6 }'
1758724 685728
ws/outbox/es5 (fix/mocha-memory-leaks ✗) $ ps -aux | awk -F' ' ' /bin\/_mocha/ { print $5,$6 }'
1844740 771992
ws/outbox/es5 (fix/mocha-memory-leaks ✗) $ ps -aux | awk -F' ' ' /bin\/_mocha/ { print $5,$6 }'
1904132 820704
ws/outbox/es5 (fix/mocha-memory-leaks ✗) $ ps -aux | awk -F' ' ' /bin\/_mocha/ { print $5,$6 }'
2003460 920804
ws/outbox/es5 (fix/mocha-memory-leaks ✗) $ ps -aux | awk -F' ' ' /bin\/_mocha/ { print $5,$6 }'
2110980 1038544
ws/outbox/es5 (fix/mocha-memory-leaks ✗) $ ps -aux | awk -F' ' ' /bin\/_mocha/ { print $5,$6 }'
2110980 1038544
ws/outbox/es5 (fix/mocha-memory-leaks ✗) $ ps -aux | awk -F' ' ' /bin\/_mocha/ { print $5,$6 }'
2138628 1037376
ws/outbox/es5 (fix/mocha-memory-leaks ✗) $ ps -aux | awk -F' ' ' /bin\/_mocha/ { print $5,$6 }'
2238980 1145448

And here's what it looks like with parent.children.pop() after Module._load(request, parent)

ws/outbox/es5 (fix/mocha-memory-leaks ✗) $ ps -aux | awk -F' ' ' /bin\/_mocha/ { print $5,$6 }'
1706272 615192
ws/outbox/es5 (fix/mocha-memory-leaks ✗) $ ps -aux | awk -F' ' ' /bin\/_mocha/ { print $5,$6 }'
1794336 723104
ws/outbox/es5 (fix/mocha-memory-leaks ✗) $ ps -aux | awk -F' ' ' /bin\/_mocha/ { print $5,$6 }'
1949984 905100
ws/outbox/es5 (fix/mocha-memory-leaks ✗) $ ps -aux | awk -F' ' ' /bin\/_mocha/ { print $5,$6 }'
1900680 868288
ws/outbox/es5 (fix/mocha-memory-leaks ✔) $ ps -aux | awk -F' ' ' /bin\/_mocha/ { print $5,$6 }'
1833076 766300
ws/outbox/es5 (fix/mocha-memory-leaks ✔) $ ps -aux | awk -F' ' ' /bin\/_mocha/ { print $5,$6 }'
1833076 766300
ws/outbox/es5 (fix/mocha-memory-leaks ✗) $ ps -aux | awk -F' ' ' /bin\/_mocha/ { print $5,$6 }'
1833076 766300
ws/outbox/es5 (fix/mocha-memory-leaks ✗) $ ps -aux | awk -F' ' ' /bin\/_mocha/ { print $5,$6 }'
1825908 769788

[charlespwd]

How would it be leaking?

When doing Module._load('../foo', parent) a new Module is instantiated and parent.children gets pushed that instance, even if it is cached.

So, in proxyquire, here, here, and here. The module being loaded gets leaked onto module.

what's holding onto it?

I'm guessing the closure that proxyquire is running in and, if not removed from the parent module, the module that required proxyquire. A spec file for instance.

[thlorenz]

The only thing I can imagine is that one of your modules is kept in the require cache and keeps collecting proxyquire children somehow? Not sure if this can/should be fixed within proxyquire. Are you requiring proxyquire multiple times in your test instead of just the top of the file?

[charlespwd]

Are you requiring proxyquire multiple times in your test instead of just the top of the file?

I'm only requiring it once per file.

[thlorenz]

OK you'd need to dig through the snapshot (retainers view) to find the actual source of your problem. This is the first time I see this related to proxyquire. If you find a problem inside proxyquire and a way to fix it, we'll greatly appreciate a PR.

Quite honestly though I see a possibility that the problem lies elsewhere in your code ... hard to tell as I don't have access to it or the snapshot and neither the bandwidth to dig into it.

Keep posting any findings here and we'll try to help, but at this point there isn't much we can do.

[charlespwd]

I would find it unlikely that the problem lies in our code since we hotswapped this project by https://github.com/charlespwd/proxyquire/ (not a fork) and we don't experience any leaks.

I can comment both this line and this line and experience a memory leak.

I can comment this block and also experience a memory leak.

I even have unit tests that break if I comment the above lines covering the leak.

All this without touching a single line of our application code.

[thlorenz]

Not trying to argue about where leak is coming from, just mentioned that this is the first kind of this issue and wanted to open your eyes to the possibility.

Obviously commenting parts of proxyquire could introduce mem leaks. What we'd need to do is finding the one that's there currently (w/out commenting anything) and fixing it.

As I said if you find a way to do that we'll gladly accept a PR.

[rochdev]

@charlespwd Did you ever find the root of the issue? We have the same problem and I can reproduce it with a single spec file. Every file save when using mocha --watch results in additional modules being loaded and referenced by proxyquire, and the old ones are never discarded.

[charlespwd]

@rochdev, we ended up using my rewrite for our project. You could either use that or write your own. It wasn't too bad to do. It turns out to about 140 LOC.

Great resources: https://nodejs.org/api/modules.html#modules_caching http://fredkschott.com/post/2014/06/require-and-the-module-system/ https://github.com/charlespwd/proxyquire/blob/master/src/proxyquire.js

[rochdev]

@charlespwd Any plans to release it on npm?

[theKashey]

Sounds like this is not a problem for proxyquire, but for any other "nodejs" mocking library, as long they all work relatively the same.

[asilvas]

This is a pretty big issue for us. Any plans to address?

[theKashey]

This problem was solved multiple times in multiple ways, and not solved in the same time. Fortunately and unfortunately module cache is mutable, and there is no simple way to restore in exactly as it was, as long as mutations are non-traceable.

There are few things you can do:

• on proxyquire side: there is bug on module deletion (https://github.com/thlorenz/proxyquire/blob/master/lib/proxyquire.js#L290), where module has to remove self from own parent as well
• on user side: you can remove known module from own parent and children(there is no own children with proxyquire model of work) - https://github.com/gregberge/loadable-components/blob/8f10896058cdfd542c2aca1a982c2e14b1d9707a/packages/server/src/util.js#L1-L18
• on user side: you can safely remove any module from the cache - https://github.com/theKashey/wipeNodeCache. However such "safety"(multiple parents of a module to evict) are not required to solve the task

Well, why not to open a PR in this case? - #261