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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leak running with Jest+Sentry #8248

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

Memory leak running with Jest+Sentry #8248

kirillgroshkov opened this issue Mar 30, 2019 · 7 comments

Comments

@kirillgroshkov
Copy link

kirillgroshkov commented Mar 30, 2019

馃悰 Bug Report

We use @sentry/node in our unit tests with Jest.

We discovered that just importing @sentry/node (as const Sentry = require('@sentry/node')) 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.

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:
getsentry/sentry-javascript#1966
#8247
#6814
#6738
#7311
#6399

To Reproduce

Steps to reproduce the behavior:

Use minimal repro repository: https://github.com/kirillgroshkov/sentry-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/sentry-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
@scotthovestadt
Copy link
Contributor

scotthovestadt commented Mar 31, 2019

This isn't related directly to Sentry, but instead:
@sentry/node -> https-proxy-agent -> agent-base

This file is the culprit:
https://github.com/TooTallNate/node-agent-base/blob/master/patch-core.js

It's importing and then patching the https module in a way that creates a real memory leak when executed repeatedly (which is a test runner-specific thing but not a Jest-only problem). If you want to fix it, a PR to that repository is probably most appropriate. Just checking if it's patched already and then not patching if already done will do the trick.

Since this leak isn't related to Jest and I don't think there's a reasonable way Jest can actually fix it, I'm likely to close this issue. If you can think of something Jest could do to solve this issue, feel free to suggest.

@kirillgroshkov
Copy link
Author

Ahaa, that's a good discovery! I've read about agent-base before, here: #6814 (comment)

Then I can try to monkey-patch that module and see what happens...

If that will help - then indeed, probably this issue can be closed.

@kirillgroshkov
Copy link
Author

@scotthovestadt any chance you can share some quick tips on how I can do such discoveries myself? I believe I have many more leaking libs in my big project... How do you debug it and see which module the leak is coming from?

@scotthovestadt
Copy link
Contributor

Sure, it's a relatively manual process (could probably be automated). I just open the entry point for the import causing issues, and comment everything out. I verify no leak. I enable it's require calls one by one until the memory leak appears (according to Jest's detection), then go into that entry point. Repeat...

Jest memory leak detector is really amazing for this type of investigation. :)

@kirillgroshkov
Copy link
Author

kirillgroshkov commented Mar 31, 2019

Makes sense!

What would you do if certain combination of commented and uncommented requires fail "sometimes" (1 of 5 times)?.. I see no randomness, hanging promises, timeouts around..

ps: I run with --no-cache to be sure

@scotthovestadt
Copy link
Contributor

Usually memory leak won't be created by a combination of different requires. Sometimes more than one may have a leak, or more than one may eventually import the same source module that creates the leak. I usually just keep going down the rabbit hole until I find the source.

If it was flakey, it's likely not caused by just importing it and has to do with certain code paths, so it gets a bit harder. Before trying to narrow down the root cause I'd definitely isolate it to a single easily reproduced test file (like you did for the issues you posted here).

@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 12, 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

2 participants