From 0ecf18fc7ea573f1283cee2d4cfac87cb5dbae49 Mon Sep 17 00:00:00 2001 From: Paul Tarjan Date: Thu, 26 Feb 2026 00:27:14 +0000 Subject: [PATCH 01/10] fsmonitor: fix khash memory leak in do_handle_client The `shown` kh_str_t was freed with kh_release_str() at a point in the code only reachable in the non-trivial response path. When the client receives a trivial response, the code jumps to the `cleanup` label, skipping the kh_release_str() call entirely and leaking the hash table. Fix this by initializing `shown` to NULL and moving the cleanup to the `cleanup` label using kh_destroy_str(), which is safe to call on NULL. This ensures the hash table is freed regardless of which code path is taken. Signed-off-by: Paul Tarjan Signed-off-by: Junio C Hamano --- builtin/fsmonitor--daemon.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c index 242c594646..bc4571938c 100644 --- a/builtin/fsmonitor--daemon.c +++ b/builtin/fsmonitor--daemon.c @@ -671,7 +671,7 @@ static int do_handle_client(struct fsmonitor_daemon_state *state, const struct fsmonitor_batch *batch; struct fsmonitor_batch *remainder = NULL; intmax_t count = 0, duplicates = 0; - kh_str_t *shown; + kh_str_t *shown = NULL; int hash_ret; int do_trivial = 0; int do_flush = 0; @@ -909,8 +909,6 @@ static int do_handle_client(struct fsmonitor_daemon_state *state, total_response_len += payload.len; } - kh_release_str(shown); - pthread_mutex_lock(&state->main_lock); if (token_data->client_ref_count > 0) @@ -954,6 +952,7 @@ static int do_handle_client(struct fsmonitor_daemon_state *state, trace2_data_intmax("fsmonitor", the_repository, "response/count/duplicates", duplicates); cleanup: + kh_destroy_str(shown); strbuf_release(&response_token); strbuf_release(&requested_token_id); strbuf_release(&payload); From 4187776953fa2a053005eaca18cfb6ae2154398b Mon Sep 17 00:00:00 2001 From: Paul Tarjan Date: Thu, 26 Feb 2026 00:27:15 +0000 Subject: [PATCH 02/10] fsmonitor: fix hashmap memory leak in fsmonitor_run_daemon The `state.cookies` hashmap is initialized during daemon startup but never freed during cleanup in the `done:` label of fsmonitor_run_daemon(). Add a hashmap_clear() call to prevent this memory leak. Signed-off-by: Paul Tarjan Signed-off-by: Junio C Hamano --- builtin/fsmonitor--daemon.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c index bc4571938c..4d52622e24 100644 --- a/builtin/fsmonitor--daemon.c +++ b/builtin/fsmonitor--daemon.c @@ -1404,6 +1404,7 @@ static int fsmonitor_run_daemon(void) done: pthread_cond_destroy(&state.cookies_cond); pthread_mutex_destroy(&state.main_lock); + hashmap_clear(&state.cookies); fsm_listen__dtor(&state); fsm_health__dtor(&state); From 945f158bc7e56febcf29a0ef13eb60f5bc267501 Mon Sep 17 00:00:00 2001 From: Paul Tarjan Date: Thu, 26 Feb 2026 00:27:16 +0000 Subject: [PATCH 03/10] compat/win32: add pthread_cond_timedwait Add a pthread_cond_timedwait() implementation to the Windows pthread compatibility layer using SleepConditionVariableCS() with a millisecond timeout computed from the absolute deadline. This enables callers to use bounded waits on condition variables instead of blocking indefinitely with pthread_cond_wait(). Signed-off-by: Paul Tarjan Signed-off-by: Junio C Hamano --- compat/win32/pthread.c | 26 ++++++++++++++++++++++++++ compat/win32/pthread.h | 2 ++ 2 files changed, 28 insertions(+) diff --git a/compat/win32/pthread.c b/compat/win32/pthread.c index 7e93146963..538ef92d9d 100644 --- a/compat/win32/pthread.c +++ b/compat/win32/pthread.c @@ -66,3 +66,29 @@ int pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex) return err_win_to_posix(GetLastError()); return 0; } + +int pthread_cond_timedwait(pthread_cond_t *cond, pthread_mutex_t *mutex, + const struct timespec *abstime) +{ + struct timeval now; + long long now_ms, deadline_ms; + DWORD timeout_ms; + + gettimeofday(&now, NULL); + now_ms = (long long)now.tv_sec * 1000 + now.tv_usec / 1000; + deadline_ms = (long long)abstime->tv_sec * 1000 + + abstime->tv_nsec / 1000000; + + if (deadline_ms <= now_ms) + timeout_ms = 0; + else + timeout_ms = (DWORD)(deadline_ms - now_ms); + + if (SleepConditionVariableCS(cond, mutex, timeout_ms) == 0) { + DWORD err = GetLastError(); + if (err == ERROR_TIMEOUT) + return ETIMEDOUT; + return err_win_to_posix(err); + } + return 0; +} diff --git a/compat/win32/pthread.h b/compat/win32/pthread.h index ccacc5a53b..d80df8d12a 100644 --- a/compat/win32/pthread.h +++ b/compat/win32/pthread.h @@ -64,6 +64,8 @@ int win32_pthread_join(pthread_t *thread, void **value_ptr); pthread_t pthread_self(void); int pthread_cond_wait(pthread_cond_t *cond, pthread_mutex_t *mutex); +int pthread_cond_timedwait(pthread_cond_t *cond, pthread_mutex_t *mutex, + const struct timespec *abstime); static inline void NORETURN pthread_exit(void *ret) { From 25351303cdb136646030ffb0bf0ebc7b82d90571 Mon Sep 17 00:00:00 2001 From: Paul Tarjan Date: Thu, 26 Feb 2026 00:27:17 +0000 Subject: [PATCH 04/10] fsmonitor: use pthread_cond_timedwait for cookie wait The cookie wait in with_lock__wait_for_cookie() uses an infinite pthread_cond_wait() loop. The existing comment notes the desire to switch to pthread_cond_timedwait(), but the routine was not available in git thread-utils. On certain container or overlay filesystems, inotify watches may succeed but events are never delivered. In this case the daemon would hang indefinitely waiting for the cookie event, which in turn causes the client to hang. Replace the infinite wait with a one-second timeout using pthread_cond_timedwait(). If the timeout fires, report an error and let the client proceed with a trivial (full-scan) response rather than blocking forever. Signed-off-by: Paul Tarjan Signed-off-by: Junio C Hamano --- builtin/fsmonitor--daemon.c | 37 ++++++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c index 4d52622e24..110fe5fb55 100644 --- a/builtin/fsmonitor--daemon.c +++ b/builtin/fsmonitor--daemon.c @@ -197,20 +197,31 @@ static enum fsmonitor_cookie_item_result with_lock__wait_for_cookie( unlink(cookie_pathname.buf); /* - * Technically, this is an infinite wait (well, unless another - * thread sends us an abort). I'd like to change this to - * use `pthread_cond_timedwait()` and return an error/timeout - * and let the caller do the trivial response thing, but we - * don't have that routine in our thread-utils. - * - * After extensive beta testing I'm not really worried about - * this. Also note that the above open() and unlink() calls - * will cause at least two FS events on that path, so the odds - * of getting stuck are pretty slim. + * Wait for the listener thread to observe the cookie file. + * Time out after a short interval so that the client + * does not hang forever if the filesystem does not deliver + * events (e.g., on certain container/overlay filesystems + * where inotify watches succeed but events never arrive). */ - while (cookie->result == FCIR_INIT) - pthread_cond_wait(&state->cookies_cond, - &state->main_lock); + { + struct timeval now; + struct timespec ts; + int err = 0; + + gettimeofday(&now, NULL); + ts.tv_sec = now.tv_sec + 1; + ts.tv_nsec = now.tv_usec * 1000; + + while (cookie->result == FCIR_INIT && !err) + err = pthread_cond_timedwait(&state->cookies_cond, + &state->main_lock, + &ts); + if (err == ETIMEDOUT && cookie->result == FCIR_INIT) { + trace_printf_key(&trace_fsmonitor, + "cookie_wait timed out"); + cookie->result = FCIR_ERROR; + } + } done: hashmap_remove(&state->cookies, &cookie->entry, NULL); From 72b1daeb13b58766356b18ccbe33f2606347553b Mon Sep 17 00:00:00 2001 From: Paul Tarjan Date: Thu, 26 Feb 2026 00:27:18 +0000 Subject: [PATCH 05/10] fsmonitor: deduplicate IPC path logic for Unix platforms The macOS fsm-ipc-darwin.c is applicable to other Unix variants as well. Rename it to fsm-ipc-unix.c and add a worktree NULL check (BUG guard) that was missing from the macOS version. To support this, introduce FSMONITOR_OS_SETTINGS which is set to "unix" for both macOS and Linux, distinct from FSMONITOR_DAEMON_BACKEND which remains platform-specific (darwin, linux, win32). Move fsm-path-utils from FSMONITOR_OS_SETTINGS to FSMONITOR_DAEMON_BACKEND since the path-utils files are platform-specific. Signed-off-by: Paul Tarjan Signed-off-by: Junio C Hamano --- Makefile | 8 ++++++-- compat/fsmonitor/{fsm-ipc-darwin.c => fsm-ipc-unix.c} | 4 +++- contrib/buildsystems/CMakeLists.txt | 2 +- meson.build | 7 ++++++- 4 files changed, 16 insertions(+), 5 deletions(-) rename compat/fsmonitor/{fsm-ipc-darwin.c => fsm-ipc-unix.c} (96%) diff --git a/Makefile b/Makefile index 89d8d73ec0..a6e6799f37 100644 --- a/Makefile +++ b/Makefile @@ -2323,13 +2323,17 @@ ifdef FSMONITOR_DAEMON_BACKEND COMPAT_CFLAGS += -DHAVE_FSMONITOR_DAEMON_BACKEND COMPAT_OBJS += compat/fsmonitor/fsm-listen-$(FSMONITOR_DAEMON_BACKEND).o COMPAT_OBJS += compat/fsmonitor/fsm-health-$(FSMONITOR_DAEMON_BACKEND).o - COMPAT_OBJS += compat/fsmonitor/fsm-ipc-$(FSMONITOR_DAEMON_BACKEND).o +ifeq ($(FSMONITOR_DAEMON_BACKEND),win32) + COMPAT_OBJS += compat/fsmonitor/fsm-ipc-win32.o +else + COMPAT_OBJS += compat/fsmonitor/fsm-ipc-unix.o +endif endif ifdef FSMONITOR_OS_SETTINGS COMPAT_CFLAGS += -DHAVE_FSMONITOR_OS_SETTINGS COMPAT_OBJS += compat/fsmonitor/fsm-settings-$(FSMONITOR_OS_SETTINGS).o - COMPAT_OBJS += compat/fsmonitor/fsm-path-utils-$(FSMONITOR_OS_SETTINGS).o + COMPAT_OBJS += compat/fsmonitor/fsm-path-utils-$(FSMONITOR_DAEMON_BACKEND).o endif ifdef WITH_BREAKING_CHANGES diff --git a/compat/fsmonitor/fsm-ipc-darwin.c b/compat/fsmonitor/fsm-ipc-unix.c similarity index 96% rename from compat/fsmonitor/fsm-ipc-darwin.c rename to compat/fsmonitor/fsm-ipc-unix.c index fe149a1b37..d34a6419bc 100644 --- a/compat/fsmonitor/fsm-ipc-darwin.c +++ b/compat/fsmonitor/fsm-ipc-unix.c @@ -27,13 +27,15 @@ const char *fsmonitor_ipc__get_path(struct repository *r) if (ipc_path) return ipc_path; - /* By default the socket file is created in the .git directory */ if (fsmonitor__is_fs_remote(r->gitdir) < 1) { ipc_path = fsmonitor_ipc__get_default_path(); return ipc_path; } + if (!r->worktree) + BUG("repository has no worktree"); + git_SHA1_Init(&sha1ctx); git_SHA1_Update(&sha1ctx, r->worktree, strlen(r->worktree)); git_SHA1_Final(hash, &sha1ctx); diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index 28877feb9d..32ef6ebe1b 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -303,7 +303,7 @@ if(SUPPORTS_SIMPLE_IPC) add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND) list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-darwin.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-health-darwin.c) - list(APPEND compat_SOURCES compat/fsmonitor/fsm-ipc-darwin.c) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-ipc-unix.c) list(APPEND compat_SOURCES compat/fsmonitor/fsm-path-utils-darwin.c) add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS) diff --git a/meson.build b/meson.build index dd52efd1c8..8de795f9d4 100644 --- a/meson.build +++ b/meson.build @@ -1332,11 +1332,16 @@ if fsmonitor_backend != '' libgit_sources += [ 'compat/fsmonitor/fsm-health-' + fsmonitor_backend + '.c', - 'compat/fsmonitor/fsm-ipc-' + fsmonitor_backend + '.c', 'compat/fsmonitor/fsm-listen-' + fsmonitor_backend + '.c', 'compat/fsmonitor/fsm-path-utils-' + fsmonitor_backend + '.c', 'compat/fsmonitor/fsm-settings-' + fsmonitor_backend + '.c', ] + + if fsmonitor_backend == 'win32' + libgit_sources += 'compat/fsmonitor/fsm-ipc-win32.c' + else + libgit_sources += 'compat/fsmonitor/fsm-ipc-unix.c' + endif endif build_options_config.set_quoted('FSMONITOR_DAEMON_BACKEND', fsmonitor_backend) build_options_config.set_quoted('FSMONITOR_OS_SETTINGS', fsmonitor_backend) From 0e210668c7edfc725a8b83df3cd342498c3a03b4 Mon Sep 17 00:00:00 2001 From: Paul Tarjan Date: Thu, 26 Feb 2026 00:27:19 +0000 Subject: [PATCH 06/10] fsmonitor: deduplicate settings logic for Unix platforms The macOS fsm-settings-darwin.c is applicable to other Unix variants as well. Rename it to fsm-settings-unix.c, using the safer xstrdup()+dirname() approach and including the "vfat" filesystem check. Now that both fsm-ipc and fsm-settings use the "unix" variant name, set FSMONITOR_OS_SETTINGS to "unix" for macOS in config.mak.uname and remove the if/else conditionals from the build files. Signed-off-by: Paul Tarjan Signed-off-by: Junio C Hamano --- Makefile | 6 +---- ...-settings-darwin.c => fsm-settings-unix.c} | 24 +++++++++++------- config.mak.uname | 2 +- contrib/buildsystems/CMakeLists.txt | 25 +++++++++---------- meson.build | 14 +++++------ 5 files changed, 35 insertions(+), 36 deletions(-) rename compat/fsmonitor/{fsm-settings-darwin.c => fsm-settings-unix.c} (82%) diff --git a/Makefile b/Makefile index a6e6799f37..54b639f919 100644 --- a/Makefile +++ b/Makefile @@ -2323,15 +2323,11 @@ ifdef FSMONITOR_DAEMON_BACKEND COMPAT_CFLAGS += -DHAVE_FSMONITOR_DAEMON_BACKEND COMPAT_OBJS += compat/fsmonitor/fsm-listen-$(FSMONITOR_DAEMON_BACKEND).o COMPAT_OBJS += compat/fsmonitor/fsm-health-$(FSMONITOR_DAEMON_BACKEND).o -ifeq ($(FSMONITOR_DAEMON_BACKEND),win32) - COMPAT_OBJS += compat/fsmonitor/fsm-ipc-win32.o -else - COMPAT_OBJS += compat/fsmonitor/fsm-ipc-unix.o -endif endif ifdef FSMONITOR_OS_SETTINGS COMPAT_CFLAGS += -DHAVE_FSMONITOR_OS_SETTINGS + COMPAT_OBJS += compat/fsmonitor/fsm-ipc-$(FSMONITOR_OS_SETTINGS).o COMPAT_OBJS += compat/fsmonitor/fsm-settings-$(FSMONITOR_OS_SETTINGS).o COMPAT_OBJS += compat/fsmonitor/fsm-path-utils-$(FSMONITOR_DAEMON_BACKEND).o endif diff --git a/compat/fsmonitor/fsm-settings-darwin.c b/compat/fsmonitor/fsm-settings-unix.c similarity index 82% rename from compat/fsmonitor/fsm-settings-darwin.c rename to compat/fsmonitor/fsm-settings-unix.c index a382590635..27d89207af 100644 --- a/compat/fsmonitor/fsm-settings-darwin.c +++ b/compat/fsmonitor/fsm-settings-unix.c @@ -5,7 +5,7 @@ #include "fsmonitor-settings.h" #include "fsmonitor-path-utils.h" - /* +/* * For the builtin FSMonitor, we create the Unix domain socket for the * IPC in the .git directory. If the working directory is remote, * then the socket will be created on the remote file system. This @@ -22,25 +22,31 @@ * The builtin FSMonitor uses a Unix domain socket in the .git * directory for IPC. These Windows drive formats do not support * Unix domain sockets, so mark them as incompatible for the daemon. - * */ static enum fsmonitor_reason check_uds_volume(struct repository *r) { struct fs_info fs; const char *ipc_path = fsmonitor_ipc__get_path(r); - struct strbuf path = STRBUF_INIT; - strbuf_add(&path, ipc_path, strlen(ipc_path)); + char *path; + char *dir; - if (fsmonitor__get_fs_info(dirname(path.buf), &fs) == -1) { - strbuf_release(&path); + /* + * Create a copy for dirname() since it may modify its argument. + */ + path = xstrdup(ipc_path); + dir = dirname(path); + + if (fsmonitor__get_fs_info(dir, &fs) == -1) { + free(path); return FSMONITOR_REASON_ERROR; } - strbuf_release(&path); + free(path); if (fs.is_remote || - !strcmp(fs.typename, "msdos") || - !strcmp(fs.typename, "ntfs")) { + !strcmp(fs.typename, "msdos") || + !strcmp(fs.typename, "ntfs") || + !strcmp(fs.typename, "vfat")) { free(fs.typename); return FSMONITOR_REASON_NOSOCKETS; } diff --git a/config.mak.uname b/config.mak.uname index 1691c6ae6e..00bcb84cee 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -178,7 +178,7 @@ ifeq ($(uname_S),Darwin) ifndef NO_PTHREADS ifndef NO_UNIX_SOCKETS FSMONITOR_DAEMON_BACKEND = darwin - FSMONITOR_OS_SETTINGS = darwin + FSMONITOR_OS_SETTINGS = unix endif endif diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index 32ef6ebe1b..4099f9a951 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -291,23 +291,22 @@ endif() if(SUPPORTS_SIMPLE_IPC) if(CMAKE_SYSTEM_NAME STREQUAL "Windows") - add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND) - list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-win32.c) - list(APPEND compat_SOURCES compat/fsmonitor/fsm-health-win32.c) - list(APPEND compat_SOURCES compat/fsmonitor/fsm-ipc-win32.c) - list(APPEND compat_SOURCES compat/fsmonitor/fsm-path-utils-win32.c) - - add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS) - list(APPEND compat_SOURCES compat/fsmonitor/fsm-settings-win32.c) + set(FSMONITOR_DAEMON_BACKEND "win32") + set(FSMONITOR_OS_SETTINGS "win32") elseif(CMAKE_SYSTEM_NAME STREQUAL "Darwin") + set(FSMONITOR_DAEMON_BACKEND "darwin") + set(FSMONITOR_OS_SETTINGS "unix") + endif() + + if(FSMONITOR_DAEMON_BACKEND) add_compile_definitions(HAVE_FSMONITOR_DAEMON_BACKEND) - list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-darwin.c) - list(APPEND compat_SOURCES compat/fsmonitor/fsm-health-darwin.c) - list(APPEND compat_SOURCES compat/fsmonitor/fsm-ipc-unix.c) - list(APPEND compat_SOURCES compat/fsmonitor/fsm-path-utils-darwin.c) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-listen-${FSMONITOR_DAEMON_BACKEND}.c) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-health-${FSMONITOR_DAEMON_BACKEND}.c) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-path-utils-${FSMONITOR_DAEMON_BACKEND}.c) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-ipc-${FSMONITOR_OS_SETTINGS}.c) add_compile_definitions(HAVE_FSMONITOR_OS_SETTINGS) - list(APPEND compat_SOURCES compat/fsmonitor/fsm-settings-darwin.c) + list(APPEND compat_SOURCES compat/fsmonitor/fsm-settings-${FSMONITOR_OS_SETTINGS}.c) endif() endif() diff --git a/meson.build b/meson.build index 8de795f9d4..589624f399 100644 --- a/meson.build +++ b/meson.build @@ -1320,10 +1320,13 @@ else endif fsmonitor_backend = '' +fsmonitor_os = '' if host_machine.system() == 'windows' fsmonitor_backend = 'win32' + fsmonitor_os = 'win32' elif host_machine.system() == 'darwin' fsmonitor_backend = 'darwin' + fsmonitor_os = 'unix' libgit_dependencies += dependency('CoreServices') endif if fsmonitor_backend != '' @@ -1334,17 +1337,12 @@ if fsmonitor_backend != '' 'compat/fsmonitor/fsm-health-' + fsmonitor_backend + '.c', 'compat/fsmonitor/fsm-listen-' + fsmonitor_backend + '.c', 'compat/fsmonitor/fsm-path-utils-' + fsmonitor_backend + '.c', - 'compat/fsmonitor/fsm-settings-' + fsmonitor_backend + '.c', + 'compat/fsmonitor/fsm-ipc-' + fsmonitor_os + '.c', + 'compat/fsmonitor/fsm-settings-' + fsmonitor_os + '.c', ] - - if fsmonitor_backend == 'win32' - libgit_sources += 'compat/fsmonitor/fsm-ipc-win32.c' - else - libgit_sources += 'compat/fsmonitor/fsm-ipc-unix.c' - endif endif build_options_config.set_quoted('FSMONITOR_DAEMON_BACKEND', fsmonitor_backend) -build_options_config.set_quoted('FSMONITOR_OS_SETTINGS', fsmonitor_backend) +build_options_config.set_quoted('FSMONITOR_OS_SETTINGS', fsmonitor_os) if not get_option('b_sanitize').contains('address') and get_option('regex').allowed() and compiler.has_header('regex.h') and compiler.get_define('REG_STARTEND', prefix: '#include ') != '' build_options_config.set('NO_REGEX', '') From 149c19ba1bb8df66fc317063a329a6d9ce1733ff Mon Sep 17 00:00:00 2001 From: Paul Tarjan Date: Thu, 26 Feb 2026 00:27:20 +0000 Subject: [PATCH 07/10] fsmonitor: implement filesystem change listener for Linux Implement the built-in fsmonitor daemon for Linux using the inotify API, bringing it to feature parity with the existing Windows and macOS implementations. The implementation uses inotify rather than fanotify because fanotify requires either CAP_SYS_ADMIN or CAP_PERFMON capabilities, making it unsuitable for an unprivileged user-space daemon. While inotify has the limitation of requiring a separate watch on every directory (unlike macOS's FSEvents, which can monitor an entire directory tree with a single watch), it operates without elevated privileges and provides the per-file event granularity needed for fsmonitor. The listener uses inotify_init1(O_NONBLOCK) with a poll loop that checks for events with a 50-millisecond timeout, keeping the inotify queue well-drained to minimize the risk of overflows. Bidirectional hashmaps map between watch descriptors and directory paths for efficient event resolution. Directory renames are tracked using inotify's cookie mechanism to correlate IN_MOVED_FROM and IN_MOVED_TO event pairs; a periodic check detects stale renames where the matching IN_MOVED_TO never arrived, forcing a resync. New directory creation triggers recursive watch registration to ensure all subdirectories are monitored. The IN_MASK_CREATE flag is used where available to prevent modifying existing watches, with a fallback for older kernels. When IN_MASK_CREATE is available and inotify_add_watch returns EEXIST, it means another thread or recursive scan has already registered the watch, so it is safe to ignore. Remote filesystem detection uses statfs() to identify network-mounted filesystems (NFS, CIFS, SMB, FUSE, etc.) via their magic numbers. Mount point information is read from /proc/mounts and matched against the statfs f_fsid to get accurate, human-readable filesystem type names for logging. When the .git directory is on a remote filesystem, the IPC socket falls back to $HOME or a user-configured directory via the fsmonitor.socketDir setting. Based-on-patch-by: Eric DeCosta Based-on-patch-by: Marziyeh Esipreh Signed-off-by: Paul Tarjan Signed-off-by: Junio C Hamano --- Documentation/config/fsmonitor--daemon.adoc | 4 +- Documentation/git-fsmonitor--daemon.adoc | 28 +- compat/fsmonitor/fsm-health-linux.c | 33 + compat/fsmonitor/fsm-listen-linux.c | 746 ++++++++++++++++++++ compat/fsmonitor/fsm-path-utils-linux.c | 220 ++++++ config.mak.uname | 10 + contrib/buildsystems/CMakeLists.txt | 4 + meson.build | 4 + 8 files changed, 1043 insertions(+), 6 deletions(-) create mode 100644 compat/fsmonitor/fsm-health-linux.c create mode 100644 compat/fsmonitor/fsm-listen-linux.c create mode 100644 compat/fsmonitor/fsm-path-utils-linux.c diff --git a/Documentation/config/fsmonitor--daemon.adoc b/Documentation/config/fsmonitor--daemon.adoc index 671f9b9462..6f8386e291 100644 --- a/Documentation/config/fsmonitor--daemon.adoc +++ b/Documentation/config/fsmonitor--daemon.adoc @@ -4,8 +4,8 @@ fsmonitor.allowRemote:: behavior. Only respected when `core.fsmonitor` is set to `true`. fsmonitor.socketDir:: - This Mac OS-specific option, if set, specifies the directory in + This Mac OS and Linux-specific option, if set, specifies the directory in which to create the Unix domain socket used for communication between the fsmonitor daemon and various Git commands. The directory must - reside on a native Mac OS filesystem. Only respected when `core.fsmonitor` + reside on a native filesystem. Only respected when `core.fsmonitor` is set to `true`. diff --git a/Documentation/git-fsmonitor--daemon.adoc b/Documentation/git-fsmonitor--daemon.adoc index 8fe5241b08..12fa866a64 100644 --- a/Documentation/git-fsmonitor--daemon.adoc +++ b/Documentation/git-fsmonitor--daemon.adoc @@ -76,9 +76,9 @@ repositories; this may be overridden by setting `fsmonitor.allowRemote` to correctly with all network-mounted repositories, so such use is considered experimental. -On Mac OS, the inter-process communication (IPC) between various Git +On Mac OS and Linux, the inter-process communication (IPC) between various Git commands and the fsmonitor daemon is done via a Unix domain socket (UDS) -- a -special type of file -- which is supported by native Mac OS filesystems, +special type of file -- which is supported by native Mac OS and Linux filesystems, but not on network-mounted filesystems, NTFS, or FAT32. Other filesystems may or may not have the needed support; the fsmonitor daemon is not guaranteed to work with these filesystems and such use is considered experimental. @@ -87,13 +87,33 @@ By default, the socket is created in the `.git` directory. However, if the `.git` directory is on a network-mounted filesystem, it will instead be created at `$HOME/.git-fsmonitor-*` unless `$HOME` itself is on a network-mounted filesystem, in which case you must set the configuration -variable `fsmonitor.socketDir` to the path of a directory on a Mac OS native +variable `fsmonitor.socketDir` to the path of a directory on a native filesystem in which to create the socket file. If none of the above directories (`.git`, `$HOME`, or `fsmonitor.socketDir`) -is on a native Mac OS file filesystem the fsmonitor daemon will report an +is on a native filesystem the fsmonitor daemon will report an error that will cause the daemon and the currently running command to exit. +LINUX CAVEATS +~~~~~~~~~~~~~ + +On Linux, the fsmonitor daemon uses inotify to monitor filesystem events. +The inotify system has per-user limits on the number of watches that can +be created. The default limit is typically 8192 watches per user. + +For large repositories with many directories, you may need to increase +this limit. Check the current limit with: + + cat /proc/sys/fs/inotify/max_user_watches + +To temporarily increase the limit: + + sudo sysctl fs.inotify.max_user_watches=65536 + +To make the change permanent, add to `/etc/sysctl.conf`: + + fs.inotify.max_user_watches=65536 + CONFIGURATION ------------- diff --git a/compat/fsmonitor/fsm-health-linux.c b/compat/fsmonitor/fsm-health-linux.c new file mode 100644 index 0000000000..43d67c4b8b --- /dev/null +++ b/compat/fsmonitor/fsm-health-linux.c @@ -0,0 +1,33 @@ +#include "git-compat-util.h" +#include "config.h" +#include "fsmonitor-ll.h" +#include "fsm-health.h" +#include "fsmonitor--daemon.h" + +/* + * The Linux fsmonitor implementation uses inotify which has its own + * mechanisms for detecting filesystem unmount and other events that + * would require the daemon to shutdown. Therefore, we don't need + * a separate health thread like Windows does. + * + * These stub functions satisfy the interface requirements. + */ + +int fsm_health__ctor(struct fsmonitor_daemon_state *state UNUSED) +{ + return 0; +} + +void fsm_health__dtor(struct fsmonitor_daemon_state *state UNUSED) +{ + return; +} + +void fsm_health__loop(struct fsmonitor_daemon_state *state UNUSED) +{ + return; +} + +void fsm_health__stop_async(struct fsmonitor_daemon_state *state UNUSED) +{ +} diff --git a/compat/fsmonitor/fsm-listen-linux.c b/compat/fsmonitor/fsm-listen-linux.c new file mode 100644 index 0000000000..36a701c06a --- /dev/null +++ b/compat/fsmonitor/fsm-listen-linux.c @@ -0,0 +1,746 @@ +#include "git-compat-util.h" +#include "dir.h" +#include "fsmonitor-ll.h" +#include "fsm-listen.h" +#include "fsmonitor--daemon.h" +#include "fsmonitor-path-utils.h" +#include "gettext.h" +#include "simple-ipc.h" +#include "string-list.h" +#include "trace.h" + +#include + +/* + * Safe value to bitwise OR with rest of mask for + * kernels that do not support IN_MASK_CREATE + */ +#ifndef IN_MASK_CREATE +#define IN_MASK_CREATE 0x00000000 +#endif + +enum shutdown_reason { + SHUTDOWN_CONTINUE = 0, + SHUTDOWN_STOP, + SHUTDOWN_ERROR, + SHUTDOWN_FORCE +}; + +struct watch_entry { + struct hashmap_entry ent; + int wd; + uint32_t cookie; + const char *dir; +}; + +struct rename_entry { + struct hashmap_entry ent; + time_t whence; + uint32_t cookie; + const char *dir; +}; + +struct fsm_listen_data { + int fd_inotify; + enum shutdown_reason shutdown; + struct hashmap watches; + struct hashmap renames; + struct hashmap revwatches; +}; + +static int watch_entry_cmp(const void *cmp_data UNUSED, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, + const void *keydata UNUSED) +{ + const struct watch_entry *e1, *e2; + + e1 = container_of(eptr, const struct watch_entry, ent); + e2 = container_of(entry_or_key, const struct watch_entry, ent); + return e1->wd != e2->wd; +} + +static int revwatches_entry_cmp(const void *cmp_data UNUSED, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, + const void *keydata UNUSED) +{ + const struct watch_entry *e1, *e2; + + e1 = container_of(eptr, const struct watch_entry, ent); + e2 = container_of(entry_or_key, const struct watch_entry, ent); + return strcmp(e1->dir, e2->dir); +} + +static int rename_entry_cmp(const void *cmp_data UNUSED, + const struct hashmap_entry *eptr, + const struct hashmap_entry *entry_or_key, + const void *keydata UNUSED) +{ + const struct rename_entry *e1, *e2; + + e1 = container_of(eptr, const struct rename_entry, ent); + e2 = container_of(entry_or_key, const struct rename_entry, ent); + return e1->cookie != e2->cookie; +} + +/* + * Register an inotify watch, add watch descriptor to path mapping + * and the reverse mapping. + */ +static int add_watch(const char *path, struct fsm_listen_data *data) +{ + const char *interned = strintern(path); + struct watch_entry *w1, *w2; + + /* add the inotify watch, don't allow watches to be modified */ + int wd = inotify_add_watch(data->fd_inotify, interned, + (IN_ALL_EVENTS | IN_ONLYDIR | IN_MASK_CREATE) + ^ IN_ACCESS ^ IN_CLOSE ^ IN_OPEN); + if (wd < 0) { + if (errno == ENOENT || errno == ENOTDIR) + return 0; /* directory was deleted or is not a directory */ + if (errno == EEXIST) + return 0; /* watch already exists, no action needed */ + if (errno == ENOSPC) + return error(_("inotify watch limit reached; " + "increase fs.inotify.max_user_watches")); + return error_errno(_("inotify_add_watch('%s') failed"), interned); + } + + /* add watch descriptor -> directory mapping */ + CALLOC_ARRAY(w1, 1); + w1->wd = wd; + w1->dir = interned; + hashmap_entry_init(&w1->ent, memhash(&w1->wd, sizeof(int))); + hashmap_add(&data->watches, &w1->ent); + + /* add directory -> watch descriptor mapping */ + CALLOC_ARRAY(w2, 1); + w2->wd = wd; + w2->dir = interned; + hashmap_entry_init(&w2->ent, strhash(w2->dir)); + hashmap_add(&data->revwatches, &w2->ent); + + return 0; +} + +/* + * Remove the inotify watch, the watch descriptor to path mapping + * and the reverse mapping. + */ +static void remove_watch(struct watch_entry *w, struct fsm_listen_data *data) +{ + struct watch_entry k1, k2, *w1, *w2; + + /* remove watch, ignore error if kernel already did it */ + if (inotify_rm_watch(data->fd_inotify, w->wd) && errno != EINVAL) + error_errno(_("inotify_rm_watch() failed")); + + k1.wd = w->wd; + hashmap_entry_init(&k1.ent, memhash(&k1.wd, sizeof(int))); + w1 = hashmap_remove_entry(&data->watches, &k1, ent, NULL); + if (!w1) + BUG("double remove of watch for '%s'", w->dir); + + if (w1->cookie) + BUG("removing watch for '%s' which has a pending rename", w1->dir); + + k2.dir = w->dir; + hashmap_entry_init(&k2.ent, strhash(k2.dir)); + w2 = hashmap_remove_entry(&data->revwatches, &k2, ent, NULL); + if (!w2) + BUG("double remove of reverse watch for '%s'", w->dir); + + /* w1->dir and w2->dir are interned strings, we don't own them */ + free(w1); + free(w2); +} + +/* + * Check for stale directory renames. + * + * https://man7.org/linux/man-pages/man7/inotify.7.html + * + * Allow for some small timeout to account for the fact that insertion of the + * IN_MOVED_FROM+IN_MOVED_TO event pair is not atomic, and the possibility that + * there may not be any IN_MOVED_TO event. + * + * If the IN_MOVED_TO event is not received within the timeout then events have + * been missed and the monitor is in an inconsistent state with respect to the + * filesystem. + */ +static int check_stale_dir_renames(struct hashmap *renames, time_t max_age) +{ + struct rename_entry *re; + struct hashmap_iter iter; + + hashmap_for_each_entry(renames, &iter, re, ent) { + if (re->whence <= max_age) + return -1; + } + return 0; +} + +/* + * Track pending renames. + * + * Tracking is done via an event cookie to watch descriptor mapping. + * + * A rename is not complete until matching an IN_MOVED_TO event is received + * for a corresponding IN_MOVED_FROM event. + */ +static void add_dir_rename(uint32_t cookie, const char *path, + struct fsm_listen_data *data) +{ + struct watch_entry k, *w; + struct rename_entry *re; + + /* lookup the watch descriptor for the given path */ + k.dir = path; + hashmap_entry_init(&k.ent, strhash(path)); + w = hashmap_get_entry(&data->revwatches, &k, ent, NULL); + if (!w) { + /* + * This can happen in rare cases where the directory was + * moved before we had a chance to add a watch on it. + * Just ignore this rename. + */ + trace_printf_key(&trace_fsmonitor, + "no watch found for rename from '%s'", path); + return; + } + w->cookie = cookie; + + /* add the pending rename to match against later */ + CALLOC_ARRAY(re, 1); + re->dir = w->dir; + re->cookie = w->cookie; + re->whence = time(NULL); + hashmap_entry_init(&re->ent, memhash(&re->cookie, sizeof(uint32_t))); + hashmap_add(&data->renames, &re->ent); +} + +/* + * Handle directory renames + * + * Once an IN_MOVED_TO event is received, lookup the rename tracking information + * via the event cookie and use this information to update the watch. + */ +static void rename_dir(uint32_t cookie, const char *path, + struct fsm_listen_data *data) +{ + struct rename_entry rek, *re; + struct watch_entry k, *w; + + /* lookup a pending rename to match */ + rek.cookie = cookie; + hashmap_entry_init(&rek.ent, memhash(&rek.cookie, sizeof(uint32_t))); + re = hashmap_get_entry(&data->renames, &rek, ent, NULL); + if (re) { + k.dir = re->dir; + hashmap_entry_init(&k.ent, strhash(k.dir)); + w = hashmap_get_entry(&data->revwatches, &k, ent, NULL); + if (w) { + w->cookie = 0; /* rename handled */ + remove_watch(w, data); + if (add_watch(path, data)) + trace_printf_key(&trace_fsmonitor, + "failed to add watch for renamed dir '%s'", + path); + } else { + /* Directory was moved out of watch tree */ + trace_printf_key(&trace_fsmonitor, + "no matching watch for rename to '%s'", path); + } + hashmap_remove_entry(&data->renames, &rek, ent, NULL); + free(re); + } else { + /* Directory was moved from outside the watch tree */ + trace_printf_key(&trace_fsmonitor, + "no matching cookie for rename to '%s'", path); + } +} + +/* + * Recursively add watches to every directory under path + */ +static int register_inotify(const char *path, + struct fsmonitor_daemon_state *state, + struct fsmonitor_batch *batch) +{ + DIR *dir; + const char *rel; + struct strbuf current = STRBUF_INIT; + struct dirent *de; + struct stat fs; + int ret = -1; + + dir = opendir(path); + if (!dir) { + if (errno == ENOENT || errno == ENOTDIR) + return 0; /* directory was deleted */ + return error_errno(_("opendir('%s') failed"), path); + } + + while ((de = readdir_skip_dot_and_dotdot(dir)) != NULL) { + strbuf_reset(¤t); + strbuf_addf(¤t, "%s/%s", path, de->d_name); + if (lstat(current.buf, &fs)) { + if (errno == ENOENT) + continue; /* file was deleted */ + error_errno(_("lstat('%s') failed"), current.buf); + goto failed; + } + + /* recurse into directory */ + if (S_ISDIR(fs.st_mode)) { + if (add_watch(current.buf, state->listen_data)) + goto failed; + if (register_inotify(current.buf, state, batch)) + goto failed; + } else if (batch) { + rel = current.buf + state->path_worktree_watch.len + 1; + trace_printf_key(&trace_fsmonitor, "explicitly adding '%s'", rel); + fsmonitor_batch__add_path(batch, rel); + } + } + ret = 0; + +failed: + strbuf_release(¤t); + if (closedir(dir) < 0) + return error_errno(_("closedir('%s') failed"), path); + return ret; +} + +static int em_rename_dir_from(uint32_t mask) +{ + return ((mask & IN_ISDIR) && (mask & IN_MOVED_FROM)); +} + +static int em_rename_dir_to(uint32_t mask) +{ + return ((mask & IN_ISDIR) && (mask & IN_MOVED_TO)); +} + +static int em_remove_watch(uint32_t mask) +{ + return (mask & IN_DELETE_SELF); +} + +static int em_dir_renamed(uint32_t mask) +{ + return ((mask & IN_ISDIR) && (mask & IN_MOVE)); +} + +static int em_dir_created(uint32_t mask) +{ + return ((mask & IN_ISDIR) && (mask & IN_CREATE)); +} + +static int em_dir_deleted(uint32_t mask) +{ + return ((mask & IN_ISDIR) && (mask & IN_DELETE)); +} + +static int em_force_shutdown(uint32_t mask) +{ + return (mask & IN_UNMOUNT) || (mask & IN_Q_OVERFLOW); +} + +static int em_ignore(uint32_t mask) +{ + return (mask & IN_IGNORED) || (mask & IN_MOVE_SELF); +} + +static void log_mask_set(const char *path, uint32_t mask) +{ + struct strbuf msg = STRBUF_INIT; + + if (mask & IN_ACCESS) + strbuf_addstr(&msg, "IN_ACCESS|"); + if (mask & IN_MODIFY) + strbuf_addstr(&msg, "IN_MODIFY|"); + if (mask & IN_ATTRIB) + strbuf_addstr(&msg, "IN_ATTRIB|"); + if (mask & IN_CLOSE_WRITE) + strbuf_addstr(&msg, "IN_CLOSE_WRITE|"); + if (mask & IN_CLOSE_NOWRITE) + strbuf_addstr(&msg, "IN_CLOSE_NOWRITE|"); + if (mask & IN_OPEN) + strbuf_addstr(&msg, "IN_OPEN|"); + if (mask & IN_MOVED_FROM) + strbuf_addstr(&msg, "IN_MOVED_FROM|"); + if (mask & IN_MOVED_TO) + strbuf_addstr(&msg, "IN_MOVED_TO|"); + if (mask & IN_CREATE) + strbuf_addstr(&msg, "IN_CREATE|"); + if (mask & IN_DELETE) + strbuf_addstr(&msg, "IN_DELETE|"); + if (mask & IN_DELETE_SELF) + strbuf_addstr(&msg, "IN_DELETE_SELF|"); + if (mask & IN_MOVE_SELF) + strbuf_addstr(&msg, "IN_MOVE_SELF|"); + if (mask & IN_UNMOUNT) + strbuf_addstr(&msg, "IN_UNMOUNT|"); + if (mask & IN_Q_OVERFLOW) + strbuf_addstr(&msg, "IN_Q_OVERFLOW|"); + if (mask & IN_IGNORED) + strbuf_addstr(&msg, "IN_IGNORED|"); + if (mask & IN_ISDIR) + strbuf_addstr(&msg, "IN_ISDIR|"); + + strbuf_strip_suffix(&msg, "|"); + + trace_printf_key(&trace_fsmonitor, "inotify_event: '%s', mask=%#8.8x %s", + path, mask, msg.buf); + + strbuf_release(&msg); +} + +int fsm_listen__ctor(struct fsmonitor_daemon_state *state) +{ + int fd; + int ret = 0; + struct fsm_listen_data *data; + + CALLOC_ARRAY(data, 1); + state->listen_data = data; + state->listen_error_code = -1; + data->fd_inotify = -1; + data->shutdown = SHUTDOWN_ERROR; + + fd = inotify_init1(O_NONBLOCK); + if (fd < 0) { + FREE_AND_NULL(state->listen_data); + return error_errno(_("inotify_init1() failed")); + } + + data->fd_inotify = fd; + + hashmap_init(&data->watches, watch_entry_cmp, NULL, 0); + hashmap_init(&data->renames, rename_entry_cmp, NULL, 0); + hashmap_init(&data->revwatches, revwatches_entry_cmp, NULL, 0); + + if (add_watch(state->path_worktree_watch.buf, data)) + ret = -1; + else if (register_inotify(state->path_worktree_watch.buf, state, NULL)) + ret = -1; + else if (state->nr_paths_watching > 1) { + if (add_watch(state->path_gitdir_watch.buf, data)) + ret = -1; + else if (register_inotify(state->path_gitdir_watch.buf, state, NULL)) + ret = -1; + } + + if (!ret) { + state->listen_error_code = 0; + data->shutdown = SHUTDOWN_CONTINUE; + } + + return ret; +} + +void fsm_listen__dtor(struct fsmonitor_daemon_state *state) +{ + struct fsm_listen_data *data; + struct hashmap_iter iter; + struct watch_entry *w; + struct watch_entry **to_remove; + size_t nr_to_remove = 0, alloc_to_remove = 0; + size_t i; + int fd; + + if (!state || !state->listen_data) + return; + + data = state->listen_data; + fd = data->fd_inotify; + + /* + * Collect all entries first, then remove them. + * We can't modify the hashmap while iterating over it. + */ + to_remove = NULL; + hashmap_for_each_entry(&data->watches, &iter, w, ent) { + ALLOC_GROW(to_remove, nr_to_remove + 1, alloc_to_remove); + to_remove[nr_to_remove++] = w; + } + + for (i = 0; i < nr_to_remove; i++) { + to_remove[i]->cookie = 0; /* ignore any pending renames */ + remove_watch(to_remove[i], data); + } + free(to_remove); + + hashmap_clear(&data->watches); + + hashmap_clear(&data->revwatches); /* remove_watch freed the entries */ + + hashmap_clear_and_free(&data->renames, struct rename_entry, ent); + + FREE_AND_NULL(state->listen_data); + + if (fd >= 0 && (close(fd) < 0)) + error_errno(_("closing inotify file descriptor failed")); +} + +void fsm_listen__stop_async(struct fsmonitor_daemon_state *state) +{ + if (state && state->listen_data && + state->listen_data->shutdown == SHUTDOWN_CONTINUE) + state->listen_data->shutdown = SHUTDOWN_STOP; +} + +/* + * Process a single inotify event and queue for publication. + */ +static int process_event(const char *path, + const struct inotify_event *event, + struct fsmonitor_batch **batch, + struct string_list *cookie_list, + struct fsmonitor_daemon_state *state) +{ + const char *rel; + const char *last_sep; + + switch (fsmonitor_classify_path_absolute(state, path)) { + case IS_INSIDE_DOT_GIT_WITH_COOKIE_PREFIX: + case IS_INSIDE_GITDIR_WITH_COOKIE_PREFIX: + /* Use just the filename of the cookie file. */ + last_sep = find_last_dir_sep(path); + string_list_append(cookie_list, + last_sep ? last_sep + 1 : path); + break; + case IS_INSIDE_DOT_GIT: + case IS_INSIDE_GITDIR: + break; + case IS_DOT_GIT: + case IS_GITDIR: + /* + * If .git directory is deleted or renamed away, + * we have to quit. + */ + if (em_dir_deleted(event->mask)) { + trace_printf_key(&trace_fsmonitor, + "event: gitdir removed"); + state->listen_data->shutdown = SHUTDOWN_FORCE; + goto done; + } + + if (em_dir_renamed(event->mask)) { + trace_printf_key(&trace_fsmonitor, + "event: gitdir renamed"); + state->listen_data->shutdown = SHUTDOWN_FORCE; + goto done; + } + break; + case IS_WORKDIR_PATH: + /* normal events in the working directory */ + if (trace_pass_fl(&trace_fsmonitor)) + log_mask_set(path, event->mask); + + if (!*batch) + *batch = fsmonitor_batch__new(); + + rel = path + state->path_worktree_watch.len + 1; + fsmonitor_batch__add_path(*batch, rel); + + if (em_dir_deleted(event->mask)) + break; + + /* received IN_MOVE_FROM, add tracking for expected IN_MOVE_TO */ + if (em_rename_dir_from(event->mask)) + add_dir_rename(event->cookie, path, state->listen_data); + + /* received IN_MOVE_TO, update watch to reflect new path */ + if (em_rename_dir_to(event->mask)) { + rename_dir(event->cookie, path, state->listen_data); + if (register_inotify(path, state, *batch)) { + state->listen_data->shutdown = SHUTDOWN_ERROR; + goto done; + } + } + + if (em_dir_created(event->mask)) { + if (add_watch(path, state->listen_data)) { + state->listen_data->shutdown = SHUTDOWN_ERROR; + goto done; + } + if (register_inotify(path, state, *batch)) { + state->listen_data->shutdown = SHUTDOWN_ERROR; + goto done; + } + } + break; + case IS_OUTSIDE_CONE: + default: + trace_printf_key(&trace_fsmonitor, + "ignoring '%s'", path); + break; + } + return 0; +done: + return -1; +} + +/* + * Read the inotify event stream and pre-process events before further + * processing and eventual publishing. + */ +static void handle_events(struct fsmonitor_daemon_state *state) +{ + /* See https://man7.org/linux/man-pages/man7/inotify.7.html */ + char buf[4096] + __attribute__ ((aligned(__alignof__(struct inotify_event)))); + + struct hashmap *watches = &state->listen_data->watches; + struct fsmonitor_batch *batch = NULL; + struct string_list cookie_list = STRING_LIST_INIT_DUP; + struct watch_entry k, *w; + struct strbuf path = STRBUF_INIT; + const struct inotify_event *event; + int fd = state->listen_data->fd_inotify; + ssize_t len; + char *ptr, *p; + + for (;;) { + len = read(fd, buf, sizeof(buf)); + if (len == -1) { + if (errno == EAGAIN || errno == EINTR) + goto done; + error_errno(_("reading inotify message stream failed")); + state->listen_data->shutdown = SHUTDOWN_ERROR; + goto done; + } + + /* nothing to read */ + if (len == 0) + goto done; + + /* Loop over all events in the buffer. */ + for (ptr = buf; ptr < buf + len; + ptr += sizeof(struct inotify_event) + event->len) { + + event = (const struct inotify_event *)ptr; + + if (em_ignore(event->mask)) + continue; + + /* File system was unmounted or event queue overflowed */ + if (em_force_shutdown(event->mask)) { + if (trace_pass_fl(&trace_fsmonitor)) + log_mask_set("Forcing shutdown", event->mask); + state->listen_data->shutdown = SHUTDOWN_FORCE; + goto done; + } + + k.wd = event->wd; + hashmap_entry_init(&k.ent, memhash(&k.wd, sizeof(int))); + + w = hashmap_get_entry(watches, &k, ent, NULL); + if (!w) { + /* Watch was removed, skip event */ + continue; + } + + /* directory watch was removed */ + if (em_remove_watch(event->mask)) { + remove_watch(w, state->listen_data); + continue; + } + + strbuf_reset(&path); + strbuf_addf(&path, "%s/%s", w->dir, event->name); + + p = fsmonitor__resolve_alias(path.buf, &state->alias); + if (!p) + p = strbuf_detach(&path, NULL); + + if (process_event(p, event, &batch, &cookie_list, state)) { + free(p); + goto done; + } + free(p); + } + strbuf_reset(&path); + fsmonitor_publish(state, batch, &cookie_list); + string_list_clear(&cookie_list, 0); + batch = NULL; + } +done: + strbuf_release(&path); + fsmonitor_batch__free_list(batch); + string_list_clear(&cookie_list, 0); +} + +/* + * Non-blocking read of the inotify events stream. The inotify fd is polled + * frequently to help minimize the number of queue overflows. + */ +void fsm_listen__loop(struct fsmonitor_daemon_state *state) +{ + int poll_num; + /* + * Interval in seconds between checks for stale directory renames. + * A directory rename that is not completed within this window + * (i.e. no matching IN_MOVED_TO for an IN_MOVED_FROM) indicates + * missed events, forcing a shutdown. + */ + const int interval = 1; + time_t checked = time(NULL); + struct pollfd fds[1]; + + fds[0].fd = state->listen_data->fd_inotify; + fds[0].events = POLLIN; + + /* + * Our fs event listener is now running, so it's safe to start + * serving client requests. + */ + ipc_server_start_async(state->ipc_server_data); + + for (;;) { + switch (state->listen_data->shutdown) { + case SHUTDOWN_CONTINUE: + poll_num = poll(fds, 1, 50); + if (poll_num == -1) { + if (errno == EINTR) + continue; + error_errno(_("polling inotify message stream failed")); + state->listen_data->shutdown = SHUTDOWN_ERROR; + continue; + } + + if ((time(NULL) - checked) >= interval) { + checked = time(NULL); + if (check_stale_dir_renames(&state->listen_data->renames, + checked - interval)) { + trace_printf_key(&trace_fsmonitor, + "missed IN_MOVED_TO events, forcing shutdown"); + state->listen_data->shutdown = SHUTDOWN_FORCE; + continue; + } + } + + if (poll_num > 0 && (fds[0].revents & POLLIN)) + handle_events(state); + + continue; + case SHUTDOWN_ERROR: + state->listen_error_code = -1; + ipc_server_stop_async(state->ipc_server_data); + break; + case SHUTDOWN_FORCE: + state->listen_error_code = 0; + ipc_server_stop_async(state->ipc_server_data); + break; + case SHUTDOWN_STOP: + default: + state->listen_error_code = 0; + break; + } + return; + } +} diff --git a/compat/fsmonitor/fsm-path-utils-linux.c b/compat/fsmonitor/fsm-path-utils-linux.c new file mode 100644 index 0000000000..b4c19e0655 --- /dev/null +++ b/compat/fsmonitor/fsm-path-utils-linux.c @@ -0,0 +1,220 @@ +#include "git-compat-util.h" +#include "fsmonitor-ll.h" +#include "fsmonitor-path-utils.h" +#include "gettext.h" +#include "trace.h" + +#include + +#ifdef HAVE_LINUX_MAGIC_H +#include +#endif + +/* + * Filesystem magic numbers for remote filesystems. + * Defined here if not available in linux/magic.h. + */ +#ifndef CIFS_SUPER_MAGIC +#define CIFS_SUPER_MAGIC 0xff534d42 +#endif +#ifndef SMB_SUPER_MAGIC +#define SMB_SUPER_MAGIC 0x517b +#endif +#ifndef SMB2_SUPER_MAGIC +#define SMB2_SUPER_MAGIC 0xfe534d42 +#endif +#ifndef NFS_SUPER_MAGIC +#define NFS_SUPER_MAGIC 0x6969 +#endif +#ifndef AFS_SUPER_MAGIC +#define AFS_SUPER_MAGIC 0x5346414f +#endif +#ifndef CODA_SUPER_MAGIC +#define CODA_SUPER_MAGIC 0x73757245 +#endif +#ifndef V9FS_MAGIC +#define V9FS_MAGIC 0x01021997 +#endif +#ifndef FUSE_SUPER_MAGIC +#define FUSE_SUPER_MAGIC 0x65735546 +#endif + +/* + * Check if filesystem type is a remote filesystem. + */ +static int is_remote_fs(unsigned long f_type) +{ + switch (f_type) { + case CIFS_SUPER_MAGIC: + case SMB_SUPER_MAGIC: + case SMB2_SUPER_MAGIC: + case NFS_SUPER_MAGIC: + case AFS_SUPER_MAGIC: + case CODA_SUPER_MAGIC: + case FUSE_SUPER_MAGIC: + return 1; + default: + return 0; + } +} + +/* + * Get the filesystem type name for logging purposes. + */ +static const char *get_fs_typename(unsigned long f_type) +{ + switch (f_type) { + case CIFS_SUPER_MAGIC: + return "cifs"; + case SMB_SUPER_MAGIC: + return "smb"; + case SMB2_SUPER_MAGIC: + return "smb2"; + case NFS_SUPER_MAGIC: + return "nfs"; + case AFS_SUPER_MAGIC: + return "afs"; + case CODA_SUPER_MAGIC: + return "coda"; + case V9FS_MAGIC: + return "9p"; + case FUSE_SUPER_MAGIC: + return "fuse"; + default: + return "unknown"; + } +} + +/* + * Find the mount point for a given path by reading /proc/mounts. + * Returns the filesystem type for the longest matching mount point. + */ +static char *find_mount(const char *path, struct statfs *fs) +{ + FILE *fp; + struct strbuf line = STRBUF_INIT; + struct strbuf match = STRBUF_INIT; + struct strbuf fstype = STRBUF_INIT; + char *result = NULL; + struct statfs path_fs; + + if (statfs(path, &path_fs) < 0) + return NULL; + + fp = fopen("/proc/mounts", "r"); + if (!fp) + return NULL; + + while (strbuf_getline(&line, fp) != EOF) { + char *fields[6]; + char *p = line.buf; + int i; + + /* Parse mount entry: device mountpoint fstype options dump pass */ + for (i = 0; i < 6 && p; i++) { + fields[i] = p; + p = strchr(p, ' '); + if (p) + *p++ = '\0'; + } + + if (i >= 3) { + const char *mountpoint = fields[1]; + const char *type = fields[2]; + struct statfs mount_fs; + + /* Check if this mount point is a prefix of our path */ + if (starts_with(path, mountpoint) && + (path[strlen(mountpoint)] == '/' || + path[strlen(mountpoint)] == '\0')) { + /* Check if filesystem ID matches */ + if (statfs(mountpoint, &mount_fs) == 0 && + !memcmp(&mount_fs.f_fsid, &path_fs.f_fsid, + sizeof(mount_fs.f_fsid))) { + /* Keep the longest matching mount point */ + if (strlen(mountpoint) > match.len) { + strbuf_reset(&match); + strbuf_addstr(&match, mountpoint); + strbuf_reset(&fstype); + strbuf_addstr(&fstype, type); + *fs = mount_fs; + } + } + } + } + } + + fclose(fp); + strbuf_release(&line); + strbuf_release(&match); + + if (fstype.len) + result = strbuf_detach(&fstype, NULL); + else + strbuf_release(&fstype); + + return result; +} + +int fsmonitor__get_fs_info(const char *path, struct fs_info *fs_info) +{ + struct statfs fs; + + if (statfs(path, &fs) == -1) { + int saved_errno = errno; + trace_printf_key(&trace_fsmonitor, "statfs('%s') failed: %s", + path, strerror(saved_errno)); + errno = saved_errno; + return -1; + } + + trace_printf_key(&trace_fsmonitor, + "statfs('%s') [type 0x%08lx]", + path, (unsigned long)fs.f_type); + + fs_info->is_remote = is_remote_fs(fs.f_type); + + /* + * Try to get filesystem type from /proc/mounts for a more + * descriptive name. + */ + fs_info->typename = find_mount(path, &fs); + if (!fs_info->typename) + fs_info->typename = xstrdup(get_fs_typename(fs.f_type)); + + trace_printf_key(&trace_fsmonitor, + "'%s' is_remote: %d, typename: %s", + path, fs_info->is_remote, fs_info->typename); + + return 0; +} + +int fsmonitor__is_fs_remote(const char *path) +{ + struct fs_info fs; + + if (fsmonitor__get_fs_info(path, &fs)) + return -1; + + free(fs.typename); + + return fs.is_remote; +} + +/* + * No-op for Linux - we don't have firmlinks like macOS. + */ +int fsmonitor__get_alias(const char *path UNUSED, + struct alias_info *info UNUSED) +{ + return 0; +} + +/* + * No-op for Linux - we don't have firmlinks like macOS. + */ +char *fsmonitor__resolve_alias(const char *path UNUSED, + const struct alias_info *info UNUSED) +{ + return NULL; +} diff --git a/config.mak.uname b/config.mak.uname index 00bcb84cee..fd91729dd2 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -68,6 +68,16 @@ ifeq ($(uname_S),Linux) BASIC_CFLAGS += -std=c99 endif LINK_FUZZ_PROGRAMS = YesPlease + + # The builtin FSMonitor on Linux builds upon Simple-IPC. Both require + # Unix domain sockets and PThreads. + ifndef NO_PTHREADS + ifndef NO_UNIX_SOCKETS + FSMONITOR_DAEMON_BACKEND = linux + FSMONITOR_OS_SETTINGS = unix + BASIC_CFLAGS += -DHAVE_LINUX_MAGIC_H + endif + endif endif ifeq ($(uname_S),GNU/kFreeBSD) HAVE_ALLOCA_H = YesPlease diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index 4099f9a951..00d6e757b6 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -296,6 +296,10 @@ if(SUPPORTS_SIMPLE_IPC) elseif(CMAKE_SYSTEM_NAME STREQUAL "Darwin") set(FSMONITOR_DAEMON_BACKEND "darwin") set(FSMONITOR_OS_SETTINGS "unix") + elseif(CMAKE_SYSTEM_NAME STREQUAL "Linux") + set(FSMONITOR_DAEMON_BACKEND "linux") + set(FSMONITOR_OS_SETTINGS "unix") + add_compile_definitions(HAVE_LINUX_MAGIC_H) endif() if(FSMONITOR_DAEMON_BACKEND) diff --git a/meson.build b/meson.build index 589624f399..0b033a9b9a 100644 --- a/meson.build +++ b/meson.build @@ -1324,6 +1324,10 @@ fsmonitor_os = '' if host_machine.system() == 'windows' fsmonitor_backend = 'win32' fsmonitor_os = 'win32' +elif host_machine.system() == 'linux' and threads.found() and compiler.has_header('linux/magic.h') + fsmonitor_backend = 'linux' + fsmonitor_os = 'unix' + libgit_c_args += '-DHAVE_LINUX_MAGIC_H' elif host_machine.system() == 'darwin' fsmonitor_backend = 'darwin' fsmonitor_os = 'unix' From 5020fb7b9005808645e01f7ddaa3b551f4ba183e Mon Sep 17 00:00:00 2001 From: Paul Tarjan Date: Thu, 26 Feb 2026 00:27:21 +0000 Subject: [PATCH 08/10] fsmonitor: add tests for Linux Add a smoke test that verifies the filesystem actually delivers inotify events to the daemon. On some configurations (e.g., overlayfs with older kernels), inotify watches succeed but events are never delivered. The daemon cookie wait will time out, but every subsequent test would fail. Skip the entire test file early when this is detected. Add a test that exercises rapid nested directory creation to verify the daemon correctly handles the EEXIST race between recursive scan and queued inotify events. When IN_MASK_CREATE is available and a directory watch is added during recursive registration, the kernel may also deliver a queued IN_CREATE event for the same directory. The second inotify_add_watch() returns EEXIST, which must be treated as harmless. An earlier version of the listener crashed in this scenario. Signed-off-by: Paul Tarjan Signed-off-by: Junio C Hamano --- t/meson.build | 8 +++- t/t7527-builtin-fsmonitor.sh | 89 +++++++++++++++++++++++++++++++++--- 2 files changed, 89 insertions(+), 8 deletions(-) diff --git a/t/meson.build b/t/meson.build index 459c52a489..bfd2f7f5a6 100644 --- a/t/meson.build +++ b/t/meson.build @@ -1208,12 +1208,18 @@ test_environment = script_environment test_environment.set('GIT_BUILD_DIR', git_build_dir) foreach integration_test : integration_tests + per_test_kwargs = test_kwargs + # The fsmonitor tests start daemon processes that in some environments + # can hang. Set a generous timeout to prevent CI from blocking. + if fs.stem(integration_test) == 't7527-builtin-fsmonitor' + per_test_kwargs += {'timeout': 1800} + endif test(fs.stem(integration_test), shell, args: [ integration_test ], workdir: meson.current_source_dir(), env: test_environment, depends: test_dependencies + bin_wrappers, - kwargs: test_kwargs, + kwargs: per_test_kwargs, ) endforeach diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh index 409cd0cd12..774da5ac60 100755 --- a/t/t7527-builtin-fsmonitor.sh +++ b/t/t7527-builtin-fsmonitor.sh @@ -10,9 +10,58 @@ then test_done fi +# Verify that the filesystem delivers events to the daemon. +# On some configurations (e.g., overlayfs with older kernels), +# inotify watches succeed but events are never delivered. The +# cookie wait will time out and the daemon logs a trace message. +# +# Use "timeout" (if available) to guard each step against hangs. +maybe_timeout () { + if type timeout >/dev/null 2>&1 + then + timeout "$@" + else + shift + "$@" + fi +} +verify_fsmonitor_works () { + git init test_fsmonitor_smoke || return 1 + + GIT_TRACE_FSMONITOR="$PWD/smoke.trace" && + export GIT_TRACE_FSMONITOR && + maybe_timeout 30 \ + git -C test_fsmonitor_smoke fsmonitor--daemon start \ + --start-timeout=10 + ret=$? + unset GIT_TRACE_FSMONITOR + if test $ret -ne 0 + then + rm -rf test_fsmonitor_smoke smoke.trace + return 1 + fi + + maybe_timeout 10 \ + test-tool -C test_fsmonitor_smoke fsmonitor-client query \ + --token 0 >/dev/null 2>&1 + maybe_timeout 5 \ + git -C test_fsmonitor_smoke fsmonitor--daemon stop 2>/dev/null + ! grep -q "cookie_wait timed out" "$PWD/smoke.trace" 2>/dev/null + ret=$? + rm -rf test_fsmonitor_smoke smoke.trace + return $ret +} + +if ! verify_fsmonitor_works +then + skip_all="filesystem does not deliver fsmonitor events (container/overlayfs?)" + test_done +fi + stop_daemon_delete_repo () { r=$1 && - test_might_fail git -C $r fsmonitor--daemon stop && + test_might_fail maybe_timeout 30 \ + git -C $r fsmonitor--daemon stop 2>/dev/null rm -rf $1 } @@ -67,7 +116,7 @@ start_daemon () { export GIT_TEST_FSMONITOR_TOKEN fi && - git $r fsmonitor--daemon start && + git $r fsmonitor--daemon start --start-timeout=10 && git $r fsmonitor--daemon status ) } @@ -520,6 +569,28 @@ test_expect_success 'directory changes to a file' ' grep "^event: dir1$" .git/trace ' +test_expect_success 'rapid nested directory creation' ' + test_when_finished "git fsmonitor--daemon stop; rm -rf rapid" && + + start_daemon --tf "$PWD/.git/trace" && + + # Rapidly create nested directories to exercise race conditions + # where directory watches may be added concurrently during + # event processing and recursive scanning. + for i in $(test_seq 1 20) + do + mkdir -p "rapid/nested/dir$i/subdir/deep" || return 1 + done && + + # Give the daemon time to process all events + sleep 1 && + + test-tool fsmonitor-client query --token 0 && + + # Verify daemon is still running (did not crash) + git fsmonitor--daemon status +' + # The next few test cases exercise the token-resync code. When filesystem # drops events (because of filesystem velocity or because the daemon isn't # polling fast enough), we need to discard the cached data (relative to the @@ -910,7 +981,10 @@ test_expect_success "submodule absorbgitdirs implicitly starts daemon" ' start_git_in_background () { git "$@" & git_pid=$! - git_pgid=$(ps -o pgid= -p $git_pid) + git_pgid=$(ps -o pgid= -p $git_pid 2>/dev/null || + awk '{print $5}' /proc/$git_pid/stat 2>/dev/null) && + git_pgid="${git_pgid## }" && + git_pgid="${git_pgid%% }" nr_tries_left=10 while true do @@ -921,15 +995,16 @@ start_git_in_background () { fi sleep 1 nr_tries_left=$(($nr_tries_left - 1)) - done >/dev/null 2>&1 & + done >/dev/null 2>&1 3>&- 4>&- 5>&- 6>&- 7>&- & watchdog_pid=$! wait $git_pid } stop_git () { - while kill -0 -- -$git_pgid + test -n "$git_pgid" || return 0 + while kill -0 -- -$git_pgid 2>/dev/null do - kill -- -$git_pgid + kill -- -$git_pgid 2>/dev/null sleep 1 done } @@ -944,7 +1019,7 @@ stop_watchdog () { test_expect_success !MINGW "submodule implicitly starts daemon by pull" ' test_atexit "stop_watchdog" && - test_when_finished "stop_git; rm -rf cloned super sub" && + test_when_finished "set +m; stop_git; rm -rf cloned super sub" && create_super super && create_sub sub && From cd26f3d67db6bd7ea7fa596cf8f283099a7d8b26 Mon Sep 17 00:00:00 2001 From: Paul Tarjan Date: Thu, 26 Feb 2026 00:27:22 +0000 Subject: [PATCH 09/10] run-command: add close_fd_above_stderr option Add a new option to struct child_process that closes file descriptors 3 and above in the child after forking but before exec. This prevents long-running child processes from inheriting pipe endpoints or other descriptors from the parent environment. The upper bound for the fd scan comes from sysconf(_SC_OPEN_MAX), capped at 4096 to avoid excessive iteration when the limit is set very high. Signed-off-by: Paul Tarjan Signed-off-by: Junio C Hamano --- run-command.c | 11 +++++++++++ run-command.h | 9 +++++++++ 2 files changed, 20 insertions(+) diff --git a/run-command.c b/run-command.c index e3e02475cc..cbadcf5ff8 100644 --- a/run-command.c +++ b/run-command.c @@ -832,6 +832,17 @@ fail_pipe: child_close(cmd->out); } + if (cmd->close_fd_above_stderr) { + long max_fd = sysconf(_SC_OPEN_MAX); + int fd; + if (max_fd < 0 || max_fd > 4096) + max_fd = 4096; + for (fd = 3; fd < max_fd; fd++) { + if (fd != child_notifier) + close(fd); + } + } + if (cmd->dir && chdir(cmd->dir)) child_die(CHILD_ERR_CHDIR); diff --git a/run-command.h b/run-command.h index 0df25e445f..a1aa1b1069 100644 --- a/run-command.h +++ b/run-command.h @@ -141,6 +141,15 @@ struct child_process { unsigned stdout_to_stderr:1; unsigned clean_on_exit:1; unsigned wait_after_clean:1; + + /** + * Close file descriptors 3 and above in the child after forking + * but before exec. This prevents the long-running child from + * inheriting pipe endpoints or other descriptors from the parent + * environment (e.g., the test harness). + */ + unsigned close_fd_above_stderr:1; + void (*clean_on_exit_handler)(struct child_process *process); }; From d6426ff9d97eda3c14942255bc1e83c6d6d2c754 Mon Sep 17 00:00:00 2001 From: Paul Tarjan Date: Thu, 26 Feb 2026 00:27:23 +0000 Subject: [PATCH 10/10] fsmonitor: close inherited file descriptors and detach in daemon When the fsmonitor daemon is spawned as a background process, it may inherit file descriptors from its parent that it does not need. In particular, when the test harness or a CI system captures output through pipes, the daemon can inherit duplicated pipe endpoints. If the daemon holds these open, the parent process never sees EOF and may appear to hang. Set close_fd_above_stderr on the child process at daemon startup so that file descriptors 3 and above are closed before any daemon work begins. This ensures the daemon does not inadvertently hold open descriptors from its launching environment. Additionally, call setsid() when the daemon starts with --detach to create a new session and process group. Without this, shells that enable job control (e.g. bash with "set -m") treat the daemon as part of the spawning command's job. Their "wait" builtin then blocks until the daemon exits, which it never does. This specifically affects systems where /bin/sh is bash (e.g. Fedora), since dash only waits for the specific PID rather than the full process group. Add a 30-second timeout to "fsmonitor--daemon stop" so it does not block indefinitely if the daemon fails to shut down. Signed-off-by: Paul Tarjan Signed-off-by: Junio C Hamano --- builtin/fsmonitor--daemon.c | 28 +++++++++++++++++++++++++--- fsmonitor-ipc.c | 3 +++ t/meson.build | 8 +------- t/t7527-builtin-fsmonitor.sh | 12 +++++++++++- 4 files changed, 40 insertions(+), 11 deletions(-) diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c index 110fe5fb55..be8f3fee1a 100644 --- a/builtin/fsmonitor--daemon.c +++ b/builtin/fsmonitor--daemon.c @@ -86,6 +86,8 @@ static int do_as_client__send_stop(void) { struct strbuf answer = STRBUF_INIT; int ret; + int max_wait_ms = 30000; + int elapsed_ms = 0; ret = fsmonitor_ipc__send_command("quit", &answer); @@ -96,8 +98,16 @@ static int do_as_client__send_stop(void) return ret; trace2_region_enter("fsm_client", "polling-for-daemon-exit", NULL); - while (fsmonitor_ipc__get_state() == IPC_STATE__LISTENING) + while (fsmonitor_ipc__get_state() == IPC_STATE__LISTENING) { + if (elapsed_ms >= max_wait_ms) { + trace2_region_leave("fsm_client", + "polling-for-daemon-exit", NULL); + return error(_("daemon did not stop within %d seconds"), + max_wait_ms / 1000); + } sleep_millisec(50); + elapsed_ms += 50; + } trace2_region_leave("fsm_client", "polling-for-daemon-exit", NULL); return 0; @@ -1431,7 +1441,7 @@ done: return err; } -static int try_to_run_foreground_daemon(int detach_console MAYBE_UNUSED) +static int try_to_run_foreground_daemon(int detach_console) { /* * Technically, we don't need to probe for an existing daemon @@ -1451,10 +1461,21 @@ static int try_to_run_foreground_daemon(int detach_console MAYBE_UNUSED) fflush(stderr); } + if (detach_console) { #ifdef GIT_WINDOWS_NATIVE - if (detach_console) FreeConsole(); +#else + /* + * Create a new session so that the daemon is detached + * from the parent's process group. This prevents + * shells with job control (e.g. bash with "set -m") + * from waiting on the daemon when they wait for a + * foreground command that implicitly spawned it. + */ + if (setsid() == -1) + warning_errno(_("setsid failed")); #endif + } return !!fsmonitor_run_daemon(); } @@ -1517,6 +1538,7 @@ static int try_to_start_background_daemon(void) cp.no_stdin = 1; cp.no_stdout = 1; cp.no_stderr = 1; + cp.close_fd_above_stderr = 1; sbgr = start_bg_command(&cp, bg_wait_cb, NULL, fsmonitor__start_timeout_sec); diff --git a/fsmonitor-ipc.c b/fsmonitor-ipc.c index f1b1631111..6112d13064 100644 --- a/fsmonitor-ipc.c +++ b/fsmonitor-ipc.c @@ -61,6 +61,9 @@ static int spawn_daemon(void) cmd.git_cmd = 1; cmd.no_stdin = 1; + cmd.no_stdout = 1; + cmd.no_stderr = 1; + cmd.close_fd_above_stderr = 1; cmd.trace2_child_class = "fsmonitor"; strvec_pushl(&cmd.args, "fsmonitor--daemon", "start", NULL); diff --git a/t/meson.build b/t/meson.build index bfd2f7f5a6..459c52a489 100644 --- a/t/meson.build +++ b/t/meson.build @@ -1208,18 +1208,12 @@ test_environment = script_environment test_environment.set('GIT_BUILD_DIR', git_build_dir) foreach integration_test : integration_tests - per_test_kwargs = test_kwargs - # The fsmonitor tests start daemon processes that in some environments - # can hang. Set a generous timeout to prevent CI from blocking. - if fs.stem(integration_test) == 't7527-builtin-fsmonitor' - per_test_kwargs += {'timeout': 1800} - endif test(fs.stem(integration_test), shell, args: [ integration_test ], workdir: meson.current_source_dir(), env: test_environment, depends: test_dependencies + bin_wrappers, - kwargs: per_test_kwargs, + kwargs: test_kwargs, ) endforeach diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh index 774da5ac60..d7e64bcb7a 100755 --- a/t/t7527-builtin-fsmonitor.sh +++ b/t/t7527-builtin-fsmonitor.sh @@ -766,7 +766,7 @@ do else test_expect_success "Matrix[uc:$uc_val][fsm:$fsm_val] enable fsmonitor" ' git config core.fsmonitor true && - git fsmonitor--daemon start && + git fsmonitor--daemon start --start-timeout=10 && git update-index --fsmonitor ' fi @@ -997,7 +997,17 @@ start_git_in_background () { nr_tries_left=$(($nr_tries_left - 1)) done >/dev/null 2>&1 3>&- 4>&- 5>&- 6>&- 7>&- & watchdog_pid=$! + + # Disable job control before wait. With "set -m", bash treats + # "wait $pid" as waiting for the entire job (process group), + # which blocks indefinitely if the fsmonitor daemon was spawned + # into the same process group and is still running. Turning off + # job control makes "wait" only wait for the specific PID. + set +m && wait $git_pid + wait_status=$? + set -m + return $wait_status } stop_git () {