@matheusbsbs / matheus-bagliano / commits / 2b08101204

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>

Jeff King committed Feb 26, 2021 at 01:14 UTC 2b08101204066cc8221afb9029c07649f67da0a0
4 files changed +39
Makefile
+7
@@ -22,6 +22,9 @@ all::
22 # when attempting to read from an fopen'ed directory (or even to fopen
23 # it at all).
24 #
25 +# Define OPEN_RETURNS_EINTR if your open() system call may return EINTR
26 +# when a signal is received (as opposed to restarting).
27 +#
28 # Define NO_OPENSSL environment variable if you do not have OpenSSL.
29 #
30 # Define USE_LIBPCRE if you have and want to use libpcre. Various
@@ -1551,6 +1554,10 @@ ifdef FREAD_READS_DIRECTORIES
1554 COMPAT_CFLAGS += -DFREAD_READS_DIRECTORIES
1555 COMPAT_OBJS += compat/fopen.o
1556 endif
1557 +ifdef OPEN_RETURNS_EINTR
1558 + COMPAT_CFLAGS += -DOPEN_RETURNS_EINTR
1559 + COMPAT_OBJS += compat/open.o
1560 +endif
1561 ifdef NO_SYMLINK_HEAD
1562 BASIC_CFLAGS += -DNO_SYMLINK_HEAD
1563 endif
compat/mingw.h
+1
@@ -227,6 +227,7 @@ int mingw_rmdir(const char *path);
227
228 int mingw_open (const char *filename, int oflags, ...);
229 #define open mingw_open
230 +#undef OPEN_RETURNS_EINTR
231
232 int mingw_fgetc(FILE *stream);
233 #define fgetc mingw_fgetc
compat/open.c new
+25
@@ -0,0 +1,25 @@
1 +#include "git-compat-util.h"
2 +
3 +#undef open
4 +int git_open_with_retry(const char *path, int flags, ...)
5 +{
6 + mode_t mode = 0;
7 + int ret;
8 +
9 + /*
10 + * Also O_TMPFILE would take a mode, but it isn't defined everywhere.
11 + * And anyway, we don't use it in our code base.
12 + */
13 + if (flags & O_CREAT) {
14 + va_list ap;
15 + va_start(ap, flags);
16 + mode = va_arg(ap, int);
17 + va_end(ap);
18 + }
19 +
20 + do {
21 + ret = open(path, flags, mode);
22 + } while (ret < 0 && errno == EINTR);
23 +
24 + return ret;
25 +}
git-compat-util.h
+6
@@ -788,6 +788,12 @@ int git_vsnprintf(char *str, size_t maxsize,
788 const char *format, va_list ap);
789 #endif
790
791 +#ifdef OPEN_RETURNS_EINTR
792 +#undef open
793 +#define open git_open_with_retry
794 +int git_open_with_retry(const char *path, int flag, ...);
795 +#endif
796 +
797 #ifdef __GLIBC_PREREQ
798 #if __GLIBC_PREREQ(2, 1)
799 #define HAVE_STRCHRNUL