[PATCH v6 10/10] fsmonitor: close inherited file descriptors and detach in daemon
- From
- Paul Tarjan via GitGitGadget <gitgitgadget@gmail.com>
- Date
- Feb 25, 2026, 20:17 UTC
- Message-ID
- <bb438afbbe3e8bbc2ed048930ef11fb02eb4163c.1772050636.git.gitgitgadget@gmail.com>
- In-Reply-To
- <pull.2147.v6.git.git.1772050636.gitgitgadget@gmail.com>
From: Paul Tarjan <github@paulisageek.com>
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 <github@paulisageek.com> --- 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 f6c406ff12..4ed848e79e 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 85ef2ae2fa..19e8306298 100644 --- a/t/meson.build +++ b/t/meson.build @@ -1210,18 +1210,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 () {
-- gitgitgadget