search

steelbrain / exec

Node's Process spawning APIs beautified
MIT License
9 stars 4 forks source link

storing unbounded child process output in memory is risky #100

Open jedwards1211 opened 5 years ago

jedwards1211 commented 5 years ago

child_process methods that return output have a maxBuffer option, to ensure that they don't run the process out of memory. I ended up implementing the same maxBuffer logic in my competing promisify-child-process package for this reason, and not storing output unless a maxBuffer option was passed.

This may be related to https://github.com/AtomLinter/linter-eslint/issues/1224#issuecomment-477299127, which is using sb-exec.

steelbrain commented 5 years ago

So here's my fly-by thought on the issue, maybe if the output you're getting is so much that it's making the machine throw out of mem errors, you shouldn't be using it in a linter which will spawn it many many times? Maybe use LSP instead?

Also like, a linter response should be just messages with error locations, It shouldn't have that much output, ever. If you have like a million errors from a single invocation, the linter should just bail out of that file by then, and presume it's minified or the config is horribly wrong for the project.

Linter's spawn process for each linting was intended only for processes that execute very quickly, and have only the relevant stuff in output. Anything more than that should be done in a daemon like LSPs.

jedwards1211 commented 5 years ago

I'm not 100% sure if it's the cause of the issue with linting. The odd thing in that linked issue is that each child_process.spawn call is taking over a second. In any case, many users of sb-exec may not consider the fact that if they naively use it to spawn a process with lots of output, it could run their process out of memory.