Re: [PATCH v7 10/10] fsmonitor: close inherited file descriptors and detach in daemon

[Paul Tarjan <paul@paultarjan.com>]

On Tue, Mar 4, 2026, Patrick Steinhardt wrote:
> Hm. We already have related logic in `daemonize()`. Should we maybe
> reuse that function, and potentially expand it to handle closing all FDs
> up to the maximum file descriptor?

daemonize() does a double-fork with setsid, which is the classic Unix
daemon pattern. But fsmonitor uses start_bg_command(), which forks the
daemon and waits for it to signal readiness over IPC. If we called
daemonize() inside the child, start_bg_command() would lose track of
the PID.

So instead we just use setsid() to detach from the terminal, and
close_fd_above_stderr in run-command to close inherited FDs before exec.

> This feels like a "while-at-it" change to me. Should it maybe be moved
> into a separate commit?

Done. Split the 30-second stop timeout into its own commit (patch 10,
"fsmonitor: add timeout to daemon stop command").

> Might make sense to reorder commits a bit so that the fix comes first.
> In that case we wouldn't ever have to introduce the timeouts in the
> first place.

Done. Reordered in v8 so run-command and daemon detach come before
the test commit. The meson timeout never appears now.

> This change feels unrelated and is not mentioned in the commit message.

--start-timeout=10 is now in the tests commit (patch 11) and documented
in that commit message.

> I thought with our call to setsid() we're not part of the same process
> group anymore. So why is this change here still needed?

setsid() runs inside the daemon after it's already been forked. The
set -m in the test is about the shell putting `git pull &` into its own
process group. Without it, the background job inherits the test
shell's pgid and `wait` stalls. Moved to the tests commit (patch 11)
with a note in the commit message explaining why it's still needed.