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

Memory leak running with Jest+Datastore #8247

Closed
kirillgroshkov opened this issue Mar 30, 2019 · 10 comments
Closed

Memory leak running with Jest+Datastore #8247

kirillgroshkov opened this issue Mar 30, 2019 · 10 comments

Comments

@kirillgroshkov
Copy link

kirillgroshkov commented Mar 30, 2019

🐛 Bug Report

We use @google-cloud/datastore in our unit tests with Jest.

We discovered that just importing @google-cloud/datastore (as const { Datastore } = require('@google-cloud/datastore')) leaks memory when running in Jest tests.

I've created a minimal repro here to showcase the issue (listed further).

Good to note that Jest has memory leak issues with many libraries, not just this one. Some popular libraries, e.g graceful-fs were "fixed" to not leak memory with Jest.

There's a symmetric issue opened in Datastore repo: googleapis/nodejs-datastore#376

Why this issue is important - it blocks us to run our test suite in our CI environment (CircleCI), which has 4Gb memory constraint. It runs out-of-memory already with ~150 test files that we have in our project. See steps to reproduce further.

Some related issues in Jest:
#8248
#6814
#6738
#7311
#6399

To Reproduce

Steps to reproduce the behavior:

Use minimal repro repository: https://github.com/kirillgroshkov/datastore-jest-leak-repro
Run npm run test-leaking or npm run test-leaking-detectLeaks to see the issue.

Expected behavior

Expected to not increase memory usage (garbage collect all memory). Similar to what happens when you run npm run test-working.

Link to repl or repo (highly encouraged)

https://github.com/kirillgroshkov/datastore-jest-leak-repro

Run npx envinfo --preset jest

Paste the results here:

  System:
    OS: macOS 10.14.4
    CPU: (8) x64 Intel(R) Core(TM) i7-4770HQ CPU @ 2.20GHz
  Binaries:
    Node: 10.15.0 - /usr/local/bin/node
    Yarn: 1.13.0 - /usr/local/bin/yarn
    npm: 6.9.0 - /usr/local/bin/npm
  npmPackages:
    jest: ^24.5.0 => 24.5.0
@rickhanlonii
Copy link
Member

Hey @kirillgroshkov thanks for filing!

Arn't these leaks within those libraries themselves? Is there a way Jest could fix leaks in app dependencies that I don't know of? Seems like the issue should be filed in those projects

@kirillgroshkov
Copy link
Author

I've opened issues both in Jest and in libraries (2 currently: Datastore and Sentry).

Indeed, if possible - libraries should fix leaks. But the problem is that there are potentially lots and lots of libraries with leaks. They're working just fine in normal environment (not test), but there's something that Jest is doing quite uniquely that makes even a tiny memory leak in library to multiply. It multiplies at least by number of tests executed (~150 files in my case), each by ~30 Mb.

Somewhere I read that in case of even tiniest memory leak (e.g holding a reference to global object with minimum possible size) Jest would hold and not release memory of much bigger size. I don't know why and what does it contain, at this point.

Just wondering - is it even possible for all libraries to fix their memory leaks to be working with Jest? Isn't there a way for Jest to somehow "release memory" after each test execution? Even if it's slower and optional (via some optional runner or so).

Memory leaks with libraries are "harmless" in production, since each library is imported only once (ensured by Node's require cache) and harm is minimized. By Jest "duplicates" it for each test and even multiplies.

@scotthovestadt
Copy link
Contributor

Thanks for the reproduction repo. I've taken a look at this and narrowed the leak down to this line, which exists in module long (many, many layers deep).

wasm = new WebAssembly.Instance(new WebAssembly.Module(new Uint8Array([
    0, 97, 115, [...truncated]
])), {}).exports;

Looks like the problem is that each time the module is executed, it's creating a new instance of a WebAssembly module, which is never cleaned up. For long in particular, this is something that wouldn't happen outside of Jest, since it's a side-effect of creating a clean slate for each test.

I don't know why it's never cleaned up. I suspect a bug in Node but I'm not sure, since I thought WebAssembly instances were supposed to be GC'd like everything else. Maybe it's just a bug with long in particular? It needs more looking into.

You can resolve your pain in the short-term by just disabling WebAssembly before the require, since the module using it is aware that it might not be available and has a fallback plan.

global.WebAssembly = undefined;

I'm not sure yet what the best solution for Jest looks like, short of just disabling WebAssembly. I put together a PoC of a module registry for WebAssembly.Instance and it seems to work, but I need some verification that it's a problem that impacts more than just this long module before I go further down that path.

Hope this helps!

@kirillgroshkov
Copy link
Author

kirillgroshkov commented Mar 30, 2019

@scotthovestadt I'll try your workaround right away!

Should I put global.WebAssembly = undefined; in my setupJest.js (setupFilesAfterEnv script)? Or somewhere else?

I'll also try to monkey-patch long package...

@kirillgroshkov
Copy link
Author

kirillgroshkov commented Mar 31, 2019

Wow! Monkey-patching long library to remove Wasm does magic! GC working as expected!

Also, putting global.WebAssembly = undefined; in setupFilesAfterEnv script works.

 PASS  src/leaking/414.leaking.test.js (160 MB heap size)
 PASS  src/leaking/226.leaking.test.js (165 MB heap size)
 PASS  src/leaking/372.leaking.test.js (171 MB heap size)
 PASS  src/leaking/319.leaking.test.js (176 MB heap size)
 PASS  src/leaking/422.leaking.test.js (181 MB heap size)
 PASS  src/leaking/36.leaking.test.js (186 MB heap size)
 PASS  src/leaking/474.leaking.test.js (191 MB heap size)
 PASS  src/leaking/115.leaking.test.js (197 MB heap size)
 PASS  src/leaking/323.leaking.test.js (202 MB heap size)
 PASS  src/leaking/348.leaking.test.js (207 MB heap size)
 PASS  src/leaking/377.leaking.test.js (212 MB heap size)
 PASS  src/leaking/118.leaking.test.js (217 MB heap size)
 PASS  src/leaking/481.leaking.test.js (61 MB heap size)
 PASS  src/leaking/240.leaking.test.js (65 MB heap size)
 PASS  src/leaking/388.leaking.test.js (71 MB heap size)
 PASS  src/leaking/338.leaking.test.js (76 MB heap size)
 PASS  src/leaking/387.leaking.test.js (81 MB heap size)
 PASS  src/leaking/247.leaking.test.js (86 MB heap size)
 PASS  src/leaking/472.leaking.test.js (91 MB heap size)
 PASS  src/leaking/465.leaking.test.js (97 MB heap size)

I'll try now on my original bigger project with 150 test files...

@sibelius
Copy link

I think we can close this

@peterdemartini
Copy link

I am still getting the memory leak in my project

@peterdemartini
Copy link

I am not sure if it is the same cause though

@kirillgroshkov
Copy link
Author

There are plenty of memory leaks with Jest and different libraries. This particular one I'll close now though, cause it was fixed with the help of @scotthovestadt

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants