Fix accidental forkbomb. #291
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
exec.CommandContext() creates a Cmd that will kill its process when its
associated Context is cancelled. Unfortunately, that kill signal
is only sent to the parent process of that Cmd, which in this case
was `sh`. Child processes can continue to run and use CPU and memory.
If your configured linter takes a while to run and uses a substantial
amount of memory (ex: eslint w/ TypeScript), and you have the default
lint configuration of linting on textDocument/didChange, then
it's quite easy to type quickly enough to forkbomb your laptop and
cause it ti run out of its many gigabytes of RAM.
This fixes that by making sure to kill the spawned lint process and
all of its children.
Note: This doesn't *only* fix the case where we run a command
within the `sh` hard-coded in the source, like this:
```
sh
+-- my_lint_command
```
But also fixes the case where the lint comand you call delegates to
other commands like this:
```
sh
+-- make
+-- pnpm
+-- eslint
```
Owner
|
Probably Windows build should be fail. |
Contributor
Author
|
Ah. Hmmm. If this were Rust I'd know how to do that check at compile-time. I don't use Go that often, though. Is there a way to skip code at compile-time, and only do the process group ID on !Windows? |
Contributor
Author
|
Aha, build constraints: Working on it! |
NfNitLoop
force-pushed
the
fix-forkbomb
branch
from
b6d0077 to
d2a39d8
Compare
Contributor
Author
|
Fixed. If there's a better way to do this in Go, let me know and I'll do it however you prefer. 😊 |
Owner
|
Well, killableCommand is not called from anywhere. |
NfNitLoop
commented
Dec 24, 2025
Owner
|
Thank you |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
exec.CommandContext() creates a Cmd that will kill its process when its associated Context is cancelled. Unfortunately, that kill signal is only sent to the parent process of that Cmd, which in this case was
sh. Child processes can continue to run and use CPU and memory.If your configured linter takes a while to run and uses a substantial amount of memory (ex: eslint w/ TypeScript), and you have the default lint configuration of linting on textDocument/didChange, then it's quite easy to type quickly enough to forkbomb your laptop and cause it ti run out of its many gigabytes of RAM.
This fixes that by making sure to kill the spawned lint process and all of its children.
Note: This doesn't only fix the case where we run a command within the
shhard-coded in the source, like this:But also fixes the case where the lint comand you call delegates to other commands like this: