From d6426ff9d97eda3c14942255bc1e83c6d6d2c754 Mon Sep 17 00:00:00 2001 From: Paul Tarjan Date: Thu, 26 Feb 2026 00:27:23 +0000 Subject: [PATCH] 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 () {