Increase available PIDs to 5#112
Merged
Merged
Conversation
Processes spawned in snekbox now have up to 5 PIDs available, each sharing the same memory limits and environment as the parent python process. As far as I could see in testing this does appear safe and processes behave as expected even when detatching from the parent or exceeding memory limits.
We define a few environment variables to stop third party libraries trying to default to spawning more processes, with the PID limit modification we can increase these values.
jb3
force-pushed
the
jb3/increase-available-pids
branch
from
b48d938 to
43e9523
Compare
jchristgit
approved these changes
Jul 17, 2021
Contributor
jchristgit
left a comment
jchristgit
left a comment
There was a problem hiding this comment.
As long as this goes into the same cgroup, this is good, Joe, very good
Contributor
|
Thank @jb3. Can you think of any way to write a test for the resource limits being shared? |
This test ensures that spawned child processes inherit the same resource group as the parent by spawning 2 child processes which each allocate a 40MB object, it then verifies that one of the child processes was killed with SIGKILL for violating the resource quota.
jb3
force-pushed
the
jb3/increase-available-pids
branch
from
5318d03 to
096a8d2
Compare
MarkKoz
approved these changes
Jul 17, 2021
Contributor
MarkKoz
left a comment
MarkKoz
left a comment
There was a problem hiding this comment.
The tests are passing, so I'm gonna go off that and say this works.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
This PR increases the PIDs cgroup limit to 5. Processes spawned in snekbox will have up to 5 PIDs available, each sharing the same memory limits and environment as the parent python process. As far as I could see in testing this does appear safe and processes behave as expected, even when detatching from the parent and so on.
I'd appreciate if anyone with a local setup could give this a shot and play with things like multiprocessing, subprocess and any other low level proc interfaces.