Only close open file descriptors #103
Open
Conversation
This file contains 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
2 tasks
chrisburr
force-pushed
the
close-fds-optimise
branch
from
ba02485 to
86d9710
Compare
chrisburr
force-pushed
the
close-fds-optimise
branch
from
86d9710 to
b240861
Compare
|
I've ran the CI on my fork and found a couple of clang-tidy complaints which have now been fixed. I've also committed b240861 which fixes the macOS CI. |
|
@DaanDeMeyer Do you have any thoughts on this? I see someone ended up debugging the same issue in #105. |
chrisburr
added a commit
to chrisburr/reproc-feedstock
that referenced
this pull request
Jul 17, 2023
chrisburr
added a commit
to chrisburr/reproc-feedstock
that referenced
this pull request
Jul 17, 2023
|
@DaanDeMeyer Is there anything missing in this PR for it to be reviewed ? |
chrisburr
added a commit
to chrisburr/reproc-feedstock
that referenced
this pull request
Oct 25, 2023
|
@DaanDeMeyer +1 for this :) |
|
This really need to be fixed |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
I've ran into issues using reproc inside containers due to the limit on the number of open file descriptors being more than 1024^2. I see the same issue has been seen by other people:
This PR removes the hard coded limit, though obviously that can make starting a subprocess extremely slow. To avoid this performance issue I've modified the code to only close open file descriptors when possible. I've tested this with macOS and Linux and it should be robust to other unix systems.
Do you have any thoughts on this approach?
cc @chaen