reproc icon indicating copy to clipboard operation
reproc copied to clipboard

Only close open file descriptors

Open chrisburr opened this issue 2 years ago • 6 comments

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:

  • https://github.com/DaanDeMeyer/reproc/issues/82
  • https://github.com/mamba-org/mamba/issues/2501

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 avatar Jun 08 '23 11:06 chrisburr

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 avatar Jun 12 '23 09:06 chrisburr

@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.

chrisburr avatar Jun 23 '23 11:06 chrisburr

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

chaen avatar Aug 01 '23 08:08 chaen

@DaanDeMeyer +1 for this :)

AntoinePrv avatar Oct 25 '23 13:10 AntoinePrv

This really need to be fixed

jcox10 avatar Jul 24 '24 21:07 jcox10