From 4c1572476985a3ca7ea616a9e769cb3b69fce893 Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Fri, 23 May 2025 14:18:37 -0400 Subject: [PATCH] gvfs-helper-client: clean up server process(es) On Linux, the following command would cause the terminal to be stuck waiting: git fetch origin foobar The issue would be that the fetch would fail with the error fatal: couldn't find remote ref foobar but the underlying git-gvfs-helper process wouldn't die. The `subprocess_exit_handler()` method would close its stdin and stdout, but that wouldn't be enough to cause the process to end, even though the `packet_read_line_gently()` call that is run in `do_sub_cmd__server()` in a loop should return -1 and the process should the terminate peacefully. While it is unclear why this does not happen, there may be other conditions where the `gvfs-helper` process would not terminate. Let's ensure that the gvfs-helper-client process cleans up the gvfs-helper server processes that it spawned upon exit. Reported-by: Stuart Wilcox Humilde Co-authored-by: Johannes Schindelin Signed-off-by: Derrick Stolee Signed-off-by: Johannes Schindelin --- gvfs-helper-client.c | 33 +++++++++++++++++++++++++++++++++ t/t9210-scalar.sh | 4 ++++ 2 files changed, 37 insertions(+) diff --git a/gvfs-helper-client.c b/gvfs-helper-client.c index c8ab947f5f9cc4..e8c1caa538e649 100644 --- a/gvfs-helper-client.c +++ b/gvfs-helper-client.c @@ -335,6 +335,36 @@ static void gh_client__choose_odb(void) } } +/* + * Custom exit handler for the `gvfs-helper` subprocesses. + * + * These helper subprocesses will keep waiting for input until they are + * stopped. The default `subprocess_exit_handler()` will instead wait for + * the subprocess to exit, which is not what we want: In case of a fatal + * error, the Git process will exit and the `gvfs-helper` subprocesses will + * need to be stopped explicitly. + * + * The default behavior of `cleanup_children()` does, however, terminate + * the process after calling the `clean_on_exit_handler`. So that's exactly + * what we do here: reproduce the exact same code as + * `subprocess_exit_handler()` modulo waiting for the process that won't + * ever terminate on its own. + */ +static void gh_client__subprocess_exit_handler(struct child_process *process) +{ + sigchain_push(SIGPIPE, SIG_IGN); + /* Closing the pipe signals the subprocess to initiate a shutdown. */ + close(process->in); + close(process->out); + sigchain_pop(SIGPIPE); + /* + * In contrast to subprocess_exit_handler(), do _not_ wait for the + * process to finish on its own accord: It needs to be terminated via + * a signal, which is what `cleanup_children()` will do after this + * function returns. + */ +} + static struct gh_server__process *gh_client__find_long_running_process( unsigned int cap_needed) { @@ -386,6 +416,9 @@ static struct gh_server__process *gh_client__find_long_running_process( &entry->subprocess, 1, &argv, gh_client__start_fn)) FREE_AND_NULL(entry); + else + entry->subprocess.process.clean_on_exit_handler = + gh_client__subprocess_exit_handler; } if (entry && diff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh index 9bfd3390d4bef3..50c8aaedaceef5 100755 --- a/t/t9210-scalar.sh +++ b/t/t9210-scalar.sh @@ -426,6 +426,10 @@ test_expect_success '`scalar clone` with GVFS-enabled server' ' ) ' +test_expect_success 'fetch does not hang in gvfs-helper' ' + test_must_fail git -C using-gvfs/src fetch origin does-not-exist +' + test_expect_success '`scalar clone --no-gvfs-protocol` skips gvfs/config' ' # the fake cache server requires fake authentication && git config --global core.askPass true &&