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

Avoid OOMs in large audit advisories #8214

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

glenjamin
Copy link

Summary

Fixes #7404 and #8012
Would supercede #7495 with the benefit of not changing the interface contract

At the moment running yarn audit --json on a project with a lot of dependencies when a common dependency like lodash is marked as vulnerable is likely to OOM.

This doesn't affect the console output, because it prints out a table per-finding, wheras the JSON output prints out a line per-advisory.

Changing the JSON output format to avoid this would arguably be quite a major breaking change, so instead i've done a bit of a trick to encode a smaller chunk of JSON output at a time, effectively streaming it.

The implementation i've done here is a little hacky, although it is contained to a small function and passes all the existing tests. I'm happy to refine the approach taken if the maintainers are aligned with the idea.

Test plan

yarn audit --json still works.

I haven't yet tested this against a repo that OOMed before, but I intend to do that today and I'm confident that it would work.

@taxonomic-blackfish
Copy link

taxonomic-blackfish commented Jul 6, 2020

This PR will still OOM:

Running yarn-local audit --json | grep auditAdvisory | jq . on a project with 56 instances of lodash (counted by rg 'lodash ' yarn.lock | wc -l) still throws an OOM after the first element is processed. My suspicion is that the OOM is caused not by a large paths field on a single auditAdvisory but by generating auditAdvisorys faster that they can be printed.

For example, yarn-local audit --json > /tmp/wat.json does not OOM but yarn-local audit --json | grep auditAdvisory > /tmp/wat.json does.

@glenjamin
Copy link
Author

Ah I see, the issue seems to be down to the yarn process not checking for backpressure on the writes to the stdout stream.

I'll see if I can fix this by checking those events and pausing output.

@pxwise
Copy link

pxwise commented Jul 9, 2020

fwiw, we were hit pretty hard by OOMs in our build process with the recent lodash vulnerability running yarn audit --json. Fix efforts here appreciated.

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 this pull request may close these issues.

yarn audit --json produces large amounts of data
3 participants