DaanDeMeyer / reproc

A cross-platform (C99/C++11) process library
MIT License
552 stars 65 forks source link

Only close open file descriptors #103

Open chrisburr opened 1 year ago

chrisburr commented 1 year ago

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

chrisburr commented 1 year ago

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 https://github.com/DaanDeMeyer/reproc/pull/103/commits/b2408614802dd0fdbf50f0e86e685a76820562fc which fixes the macOS CI.

chrisburr commented 1 year ago

@DaanDeMeyer Do you have any thoughts on this? I see someone ended up debugging the same issue in https://github.com/DaanDeMeyer/reproc/issues/105.

chaen commented 1 year ago

@DaanDeMeyer Is there anything missing in this PR for it to be reviewed ?

AntoinePrv commented 11 months ago

@DaanDeMeyer +1 for this :)

jcox10 commented 2 months ago

This really need to be fixed