HomePhabricator

remotefilelog: open pack files with O_CLOEXEC

Authored by simpkins.

Description

remotefilelog: open pack files with O_CLOEXEC

Summary:
If os.name is "posix", add the "e" flag when opening pack files. On systems
using glibc 2.7+ this causes the underlying operating system open() call to use
the O_CLOEXEC flag. This ensures that the pack file descriptors will not be
inherited by any children processes. This can be helpful since the
remotefilelog code can sometimes have thousands of pack files open. This
causes problems for children processes that expect to be able to use select()
on file descriptors they create, since they can end up with file descriptor
numbers over 1024.

While making sure to always set close_fds=True when calling subprocess.Popen()
can help with this, using O_CLOEXEC to fix the problem from the start seems
like a nicer fix.

The extra "e" flag generally should be ignored by other C library
implementations that do not support this flag. I explicitly scoped this to the
"posix" OS just to ensure that this cannot collide with other OS-specific flags
on other operating systems. (Windows also defines several of its own custom
fopen() flags. Windows does not currently use "e", but it seems best to avoid
it anyway.)

Test Plan:
Ran hg under strace and confirmed that the pack files were opened with
O_CLOEXEC.

Reviewers: quark, durham, #fbhgext

Reviewed By: quark, #fbhgext

Differential Revision: https://phab.mercurial-scm.org/D398

Details

Committed
simpkinsAug 15 2017, 1:49 PM
Reviewer
Restricted Project
Differential Revision
D398: remotefilelog: open pack files with O_CLOEXEC
Parents
rFBHGX6816754eb0f8: arc: fix performance of "arc unit" with vanilla arcanist
Branches
Unknown
Tags
Unknown