Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proxyquire is leaking on module.parent #174

Open
charlespwd opened this issue Sep 19, 2017 · 16 comments · May be fixed by #261
Open

Proxyquire is leaking on module.parent #174

charlespwd opened this issue Sep 19, 2017 · 16 comments · May be fixed by #261

Comments

@charlespwd
Copy link

charlespwd commented Sep 19, 2017

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.

image

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
Copy link
Owner

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 charlespwd changed the title Proxyquire is leaking on module.parent ES-1397 Proxyquire is leaking on module.parent Sep 19, 2017
@charlespwd charlespwd changed the title ES-1397 Proxyquire is leaking on module.parent Proxyquire is leaking on module.parent Sep 19, 2017
@charlespwd
Copy link
Author

charlespwd commented Sep 19, 2017

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
Copy link
Owner

thlorenz commented Sep 19, 2017

proxyquire itself would be leaking

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

@charlespwd
Copy link
Author

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
Copy link
Author

charlespwd commented Sep 19, 2017

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Author

charlespwd commented Sep 19, 2017

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
Copy link
Owner

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
Copy link

rochdev commented Apr 23, 2018

@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
Copy link
Author

charlespwd commented Apr 23, 2018

@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
Copy link

rochdev commented Apr 23, 2018

@charlespwd Any plans to release it on npm?

@theKashey
Copy link
Contributor

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 comment has been minimized.

theKashey added a commit to theKashey/proxyquire that referenced this issue Apr 21, 2021
@theKashey theKashey linked a pull request Apr 21, 2021 that will close this issue
@theKashey
Copy link
Contributor

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:

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants