On Wed, Jun 02, 2021 at 03:49:31PM -0400, Jeff King wrote:
> And so when he gets this error:
>
> fatal: fsync error on '.git/objects/pack/tmp_pack_NkPgqN': Interrupted system call
>
> presumably we were in fsync() when the signal arrived, and unlike most
> other platforms, the call needs to be restarted manually (even though we
> set up the signal with SA_RESTART).
Aha, thanks for explaining. That makes sense to me.
> I'm not sure if this violates POSIX or not (I couldn't find a
> definitive answer to the set of interruptible functions in the
> standard).
I couldn't find anything either, but I believe you that what you
outlined is the right solution.
> But either way, the workaround is probably something like:
Seems very reasonable. Here's a patch:
--- >8 ---
Subject: [PATCH] compat: introduce git_fsync_with_restart()
Some platforms, like NonStop do not automatically restart fsync() when
interrupted by a signal, even when that signal is setup with SA_RESTART.
This can lead to test breakage, e.g., where "--progress" is used, thus
SIGALRM is sent often, and can interrupt an fsync() syscall.
Add a Makefile knob FSYNC_NEEDS_RESTART to replace fsync() with one that
gracefully handles getting EINTR.
Reported-by: Randall S. Becker <randall.becker@xxxxxxxxxxxx>
Signed-off-by: Jeff King <peff@xxxxxxxx>
Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
---
Makefile | 8 ++++++++
compat/fsync.c | 10 ++++++++++
git-compat-util.h | 6 ++++++
3 files changed, 24 insertions(+)
create mode 100644 compat/fsync.c
diff --git a/Makefile b/Makefile
index c3565fc0f8..ebea693a2c 100644
--- a/Makefile
+++ b/Makefile
@@ -423,6 +423,9 @@ all::
# Define NEED_ACCESS_ROOT_HANDLER if access() under root may success for X_OK
# even if execution permission isn't granted for any user.
#
+# Define FSYNC_NEEDS_RESTART if your platform's fsync() needs to be restarted
+# manually when interrupted by a signal.
+#
# Define PAGER_ENV to a SP separated VAR=VAL pairs to define
# default environment variables to be passed when a pager is spawned, e.g.
#
@@ -1926,6 +1929,11 @@ ifdef NEED_ACCESS_ROOT_HANDLER
COMPAT_OBJS += compat/access.o
endif
+ifdef FSYNC_NEEDS_RESTART
+ COMPAT_CFLAGS += -DFSYNC_NEEDS_RESTART
+ COMPAT_OBJS += compat/fsync.o
+endif
+
ifeq ($(TCLTK_PATH),)
NO_TCLTK = NoThanks
endif
diff --git a/compat/fsync.c b/compat/fsync.c
new file mode 100644
index 0000000000..f340fc628b
--- /dev/null
+++ b/compat/fsync.c
@@ -0,0 +1,10 @@
+#include "git-compat-util.h"
+
+#undef fsync
+int git_fsync_with_restart(int fd)
+{
+ int ret;
+ while ((ret = fsync(fd)) < 0 && errno == EINTR)
+ ; /* try again */
+ return ret;
+}
diff --git a/git-compat-util.h b/git-compat-util.h
index a508dbe5a3..972fdb609f 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -833,6 +833,12 @@ const char *inet_ntop(int af, const void *src, char *dst, size_t size);
int git_atexit(void (*handler)(void));
#endif
+#ifdef FSYNC_NEEDS_RESTART
+#undef fsync
+#define fsync git_fsync_with_restart
+int git_fsync_with_restart(int fd);
+#endif
+
static inline size_t st_add(size_t a, size_t b)
{
if (unsigned_add_overflows(a, b))
--
2.31.1.163.ga65ce7f831
