Re: [PATCH v1 1/2] sha1_file: open window into packfiles with CLOEXEC
- From
- Eric Wong <e@80x24.org>
- Date
- Sep 5, 2016, 22:27 UTC
- Message-ID
- <20160905222715.GA30031@starla>
- In-Reply-To
- <20160905211111.72956-2-larsxschneider@gmail.com>
larsxschneider@gmail.com wrote:
> All processes that the Git main process spawns inherit the open file > descriptors of the main process. These leaked file descriptors can > cause problems.
Show 8 quoted lines
> -int git_open_noatime(const char *name)
> +int git_open_noatime_cloexec(const char *name)
> {
> - static int sha1_file_open_flag = O_NOATIME;
> + static int sha1_file_open_flag = O_NOATIME | O_CLOEXEC;
>
> for (;;) {
> int fd;If there's real problems being caused by lack of cloexec today, I think the F_SETFD fallback I proposed in https://public-inbox.org/git/20160818173555.GA29253@starla/ will be necessary.
I question the need for the "_cloexec" suffixing in the function name since the old function is going away entirely.
I prefer all FD-creating functions set cloexec by default for FD > 2 to avoid inadvertantly leaking FDs. So we ought to use pipe2, accept4, socket(..., SOCK_CLOEXEC), etc... and fallback to the racy+slower F_SETFD when not available.
Fwiw, Perl has been setting cloexec on FDs above $^F (2, $SYSTEM_FD_MAX) for decades, and Ruby started doing it a few years ago, too.