Makefile: add OPEN_RETURNS_EINTR knob

On some platforms, open() reportedly returns EINTR when opening regular
files and we receive a signal (usually SIGALRM from our progress meter).
This shouldn't happen, as open() should be a restartable syscall, and we
specify SA_RESTART when setting up the alarm handler. So it may actually
be a kernel or libc bug for this to happen. But it has been reported on
at least one version of Linux (on a network filesystem):

  https://lore.kernel.org/git/c8061cce-71e4-17bd-a56a-a5fed93804da@neanderfunk.de/

as well as on macOS starting with Big Sur even on a regular filesystem.

We can work around it by retrying open() calls that get EINTR, just as
we do for read(), etc. Since we don't ever _want_ to interrupt an open()
call, we can get away with just redefining open, rather than insisting
all callsites use xopen().

We actually do have an xopen() wrapper already (and it even does this
retry, though there's no indication of it being an observed problem back
then; it seems simply to have been lifted from xread(), etc). But it is
used hardly anywhere, and isn't suitable for general use because it will
die() on error. In theory we could combine the two, but it's awkward to
do so because of the variable-args interface of open().

This patch adds a Makefile knob for enabling the workaround. It's not
enabled by default for any platforms in config.mak.uname yet, as we
don't have enough data to decide how common this is (I have not been
able to reproduce on either Linux or Big Sur myself). It may be worth
enabling preemptively anyway, since the cost is pretty low (if we don't
see an EINTR, it's just an extra conditional).

However, note that we must not enable this on Windows. It doesn't do
anything there, and the macro overrides the existing mingw_open()
redirection. I've added a preemptive #undef here in the mingw header
(which is processed first) to just quietly disable it (we could also
make it an #error, but there is little point in being so aggressive).

Reported-by: Aleksey Kliger <alklig@microsoft.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Makefile b/Makefile
index 4128b457e14bda2a1b03bc439f413e82cf7f691a..08aa57f1dbdd7eb69f42772c5773bef158daab6d 100644 (file)
--- a/Makefile
+++ b/Makefile
@@ -22,6 +22,9 @@ all::
 # when attempting to read from an fopen'ed directory (or even to fopen
 # it at all).
 #
+# Define OPEN_RETURNS_EINTR if your open() system call may return EINTR
+# when a signal is received (as opposed to restarting).
+#
 # Define NO_OPENSSL environment variable if you do not have OpenSSL.
 #
 # Define USE_LIBPCRE if you have and want to use libpcre. Various
@@ -1551,6 +1554,10 @@ ifdef FREAD_READS_DIRECTORIES
        COMPAT_CFLAGS += -DFREAD_READS_DIRECTORIES
        COMPAT_OBJS += compat/fopen.o
 endif
+ifdef OPEN_RETURNS_EINTR
+       COMPAT_CFLAGS += -DOPEN_RETURNS_EINTR
+       COMPAT_OBJS += compat/open.o
+endif
 ifdef NO_SYMLINK_HEAD
        BASIC_CFLAGS += -DNO_SYMLINK_HEAD
 endif

diff --git a/compat/mingw.h b/compat/mingw.h
index af8eddd73edb2b1d19caddad4732aa7a370d0cc0..c9a52ad64a681fa9deff40c98d6ddabf842573de 100644 (file)
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -227,6 +227,7 @@ int mingw_rmdir(const char *path);
 
 int mingw_open (const char *filename, int oflags, ...);
 #define open mingw_open
+#undef OPEN_RETURNS_EINTR
 
 int mingw_fgetc(FILE *stream);
 #define fgetc mingw_fgetc

diff --git a/compat/open.c b/compat/open.c
new file mode 100644 (file)
index 0000000..eb3754a
--- /dev/null
+++ b/compat/open.c
@@ -0,0 +1,25 @@
+#include "git-compat-util.h"
+
+#undef open
+int git_open_with_retry(const char *path, int flags, ...)
+{
+       mode_t mode = 0;
+       int ret;
+
+       /*
+        * Also O_TMPFILE would take a mode, but it isn't defined everywhere.
+        * And anyway, we don't use it in our code base.
+        */
+       if (flags & O_CREAT) {
+               va_list ap;
+               va_start(ap, flags);
+               mode = va_arg(ap, int);
+               va_end(ap);
+       }
+
+       do {
+               ret = open(path, flags, mode);
+       } while (ret < 0 && errno == EINTR);
+
+       return ret;
+}

diff --git a/git-compat-util.h b/git-compat-util.h
index 104993b97550c8df8f3834929e945dee43af2742..67954854f1a47e85dfc1fc53b3a7488bb680b0a2 100644 (file)
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -788,6 +788,12 @@ int git_vsnprintf(char *str, size_t maxsize,
                  const char *format, va_list ap);
 #endif
 
+#ifdef OPEN_RETURNS_EINTR
+#undef open
+#define open git_open_with_retry
+int git_open_with_retry(const char *path, int flag, ...);
+#endif
+
 #ifdef __GLIBC_PREREQ
 #if __GLIBC_PREREQ(2, 1)
 #define HAVE_STRCHRNUL