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

Add Config Option to Limit Number of Processes #2085

Merged
merged 2 commits into from
Aug 11, 2024

Conversation

Syphdias
Copy link
Contributor

To protect your system and ghostty from misbehaving programs that launch too many processes for the system to handle (e.g. like a fork bomb), this implements an option to limit the number of processes that can be started in a surface.

A fork bomb for example or other misbehaving program would then only take down one surface and not the entire system.

Side node:
If I am right in issue #2084, this feature does not actually work on a per surface basis but on all surfaces. If this is the case, it could probably be fixed together. Chances are, that I am wrong though 😉

Further improvements that could be done:

  • unify way to set cgroup attributes
  • set sane default: 10% of system max?

To protect your system and ghostty from misbehaving programs that launch
too many processes for the system to handle (e.g. like a fork bomb),
this implements an option to limit the number of processes that can be
started in a surface.

A fork bomb for example or other misbehaving program would then only
take down one surface and not the entire system.

Side node:
If I am right in issue #2084, this feature does not actually work on a
per surface basis but on all surfaces. If this is the case, it could
probably be fixed together. Chances are, that I am wrong though 😉

Further improvements that could be done:
- unify way to set cgroup attributes
- set sane default: 10% of system max?
@mitchellh
Copy link
Contributor

Looks great. I think unifying the cgroup attributes is a good idea... I can look into that since it's a bit more subjective.

I think the default of not setting it at all for now is fine but some sane default in the future also makes sense. If we want to be really safe we can set it to something like 75% of max...

@mitchellh
Copy link
Contributor

Looks like querying the pids max while respecting potentially Ghostty itself being in a cgroup is a non-trivial (or at least, non performance trivial) task. For now let's keep it unset as a default.

@mitchellh mitchellh merged commit 23de1fb into ghostty-org:main Aug 11, 2024
17 of 19 checks passed
@mitchellh mitchellh deleted the option-to-limit-procs branch August 11, 2024 22:57
@Syphdias Syphdias restored the option-to-limit-procs branch August 12, 2024 08:41
@Syphdias Syphdias deleted the option-to-limit-procs branch August 12, 2024 08:41
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.

2 participants