Memory leak issue with jest

[juhosuominen]

What is the expected behavior?
Node heap size shouldn't grow in every test.

What is the actual behavior?
Heap size increases after every test file.

With nock:

PASS  test/function-86.js (22 MB heap size)
PASS  test/function-75.js (22 MB heap size)
PASS  test/function-1.js (22 MB heap size)
PASS  test/function-43.js (26 MB heap size)
PASS  test/function-24.js (26 MB heap size)
...
PASS  test/function-48.js (133 MB heap size)
PASS  test/function-82.js (137 MB heap size)
PASS  test/function-87.js (137 MB heap size)
PASS  test/function-20.js (137 MB heap size)
PASS  test/function-79.js (140 MB heap size)

Test Suites: 100 passed, 100 total
Tests:       100 passed, 100 total
Snapshots:   0 total
Time:        25.506s, estimated 26s

Without nock:

PASS  test/function-86.js (19 MB heap size)
PASS  test/function-75.js (19 MB heap size)
PASS  test/function-1.js (19 MB heap size)
PASS  test/function-65.js (19 MB heap size)
PASS  test/function-96.js (19 MB heap size)
...
PASS  test/function-93.js (20 MB heap size)
PASS  test/function-61.js (20 MB heap size)
PASS  test/function-49.js (20 MB heap size)
PASS  test/function-87.js (20 MB heap size)
PASS  test/function-79.js (20 MB heap size)

Test Suites: 100 passed, 100 total
Tests:       100 passed, 100 total
Snapshots:   0 total
Time:        27.079s

How to reproduce the issue

Even a single require/import of nock seems to cause the issue.

Minimal test case: https://github.com/juhosuominen/jest-nock-memory-leak

Versions

Software	Version(s)
Nock	11.7.0
Node	11.15.0

[mastermatt]

@juhosuominen thanks for this. I've started to play with your test repo a bit, but I'll need to spend more time with it this weekend.

One thing I've noticed is that I can't reproduce your statement "Even a single require/import of nock seems to cause the issue.".
While I do get the same results listed above using npm run test (large heap size) and npm run test:no-nock (steady heap size), if I just add require('nock'), unconditionally, to one of the tests then run npm run test:no-nock I don't see the heap get large.

So on my machine simply importing Nock isn't the issue, it's the repeated use of Nock that does it.
@juhosuominen can you confirm you are experience different?

Doing a quick dive into a heap dump points to Babel/generator, but I need to more time exploring to see if that is a red herring.

[mastermatt]

Ahh now I see what your statement means. A single require of Nock in each test suite causes the issue.
Yes, I can reproduce this. Having many Jest test suite whose contents are simply the following demonstrates the leak

require('nock');

it('should do nothing', async () => {
	// do nothing
});

So far I've been able to determine that this has to do with Jest not respecting the require.cache across suites (ref, example) and the fact that Nock mutates the global http module on import. A known shortcoming.

I'm not sure why there is a memory leak. I'm still leaning towards something related to Babel.

[paulmelnikow]

Just a hunch, any chance invoking nock.restore() after each test makes a difference?

We do keep a reference to the original http & https functions inside nock.

Though I’m still not really clear on what Jest is doing, or how the combination of Jest and Nock creates leaks.

[mastermatt]

yes, I have seen that adding nock.restore() at the end of each suite completely fixes the leak.

Right now I'm playing around with different ways of tacking on the original ClientRequest onto a global so Nock doesn't keep extending its own subclass. But I haven't figured out the quirks of Jest's and/or Babel's caching mechanisms yet.

I'm going to call it a night.

[paulmelnikow]

We end up extending our own subclass over and over?

[mastermatt]

We end up extending our own subclass over and over?

yes.

[mastermatt]

It's hard to call this a bug, since Jest is the one breaking from how Node is suppose to work.

[mastermatt]

What's confusing is that if I do the dance to store the original ClientRequest on the http lib then reuse that to inherit from, the leak is still present. It seems Babel Generator notices the class has changed and re-processes it, while keeping a ref to the old classes.

This issue also exists for the http[s].request and get methods.

[mastermatt]

The discussion in Jest jestjs/jest#6814

And a nice write up on it. https://chanind.github.io/javascript/2019/10/12/jest-tests-memory-leak.html

[paulmelnikow]

I wonder if we could detect Jest and then print a warning if we try to extend and discover that it’s already been overridden. We could also automatically reset but I don’t like that idea too much; at least not without understanding better what Jest is doing.

Another option is to avoid patching if it’s already patched by setting a flag in the global module. (Perhaps a flag works where saving original request doesn’t?) Though I wonder if that would work. Nock has its own global state, and I’m not quite clear whether request patched in a previous test would work with mocks defined in a new test.

[juhosuominen]

@mastermatt Yes, it seems I was mistaken about the "Even a single require/import of nock seems to cause the issue" part. We were probably importing nock in some test boilerplate file.

I did manage to fix the memory leak by calling nock.restore() after each test suite as you suggested.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. We try to do our best, but nock is maintained by volunteers and there is only so much we can do at a time. Thank you for your contributions.

[kirillgroshkov]

Any update on this?

[paulmelnikow]

I did manage to fix the memory leak by calling nock.restore() after each test suite as you suggested.

Isn't this the fix?

[kirillgroshkov]

I did manage to fix the memory leak by calling nock.restore() after each test suite as you suggested.

Isn't this the fix?

Yes, probably. I wasn't able to confirm it (Jest still reports leaks and actually leaks 100s of Mbs of memory per test file). But that's probably due to other dependencies I have. So, ignore my last comment please:)

[kirillgroshkov]

Just a question, will nock.cleanAll() also do the job, or only nock.restore()?

[mastermatt]

cleanAll removes out all the registered interceptors, but does not remove the monkey patched functions and class from the builtins. So it won't be enough in this case.

[kirillgroshkov]

cleanAll removes out all the registered interceptors, but does not remove the monkey patched functions and class from the builtins. So it won't be enough in this case.

Thanks!

[mainfraame]

this is still an issue. I migrated to xhr mock and my issues went away. The authors of this library should take a look at what graceful-fs and agent-base did to resolve the issue. A great thread about it can be found here

[mastermatt]

@mentierd yes, at this time we don't plan on "fixing" this as it's an issue with Jest. jestjs/jest#6814 as noted in the comment above. #1817 (comment).

We have updated our docs to describe how to avoid this when using Nock in Jest suites.
Memory issues with Jest

Unfortunately, the comment just above that one describes an attempt to get around it, in the same way graceful-fs and agent-base did. Those libs have slightly different use cases. graceful-fs was caching whole stdlib modules and agent-base was only monkey patching https.request. Nock overrides http.ClientRequest and the underlaying implementation of Babel, that Jest utilizes, maintains a reference to the child classes and reprocesses the class even when the override is skipped on subsequent activations of Nock.

If anyone comes across a way to get around this or force Babel to stop its caching in a way that doesn't make things worse, we'd love a PR.

[RedTn]

This still seems to be an issue, though I can confirm that when I use node 11.15.0 and jest ^26 that there is no leak. As soon as I upgrade to node 16 (16.10.0) this is an issue again
#2383