Skip to content

Commit 6e041f8

Browse files
authored
gvfs-helper-client: clean up server process (#756)
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. This PR addresses that by skipping the `finish_command()` call of the `clean_on_exit_handler` and instead lets `cleanup_children()` send a SIGTERM to terminate those spawned child processes.
2 parents 1e73cc0 + 4c15724 commit 6e041f8

File tree

2 files changed

+37
-0
lines changed

2 files changed

+37
-0
lines changed

gvfs-helper-client.c

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,36 @@ static void gh_client__choose_odb(void)
335335
}
336336
}
337337

338+
/*
339+
* Custom exit handler for the `gvfs-helper` subprocesses.
340+
*
341+
* These helper subprocesses will keep waiting for input until they are
342+
* stopped. The default `subprocess_exit_handler()` will instead wait for
343+
* the subprocess to exit, which is not what we want: In case of a fatal
344+
* error, the Git process will exit and the `gvfs-helper` subprocesses will
345+
* need to be stopped explicitly.
346+
*
347+
* The default behavior of `cleanup_children()` does, however, terminate
348+
* the process after calling the `clean_on_exit_handler`. So that's exactly
349+
* what we do here: reproduce the exact same code as
350+
* `subprocess_exit_handler()` modulo waiting for the process that won't
351+
* ever terminate on its own.
352+
*/
353+
static void gh_client__subprocess_exit_handler(struct child_process *process)
354+
{
355+
sigchain_push(SIGPIPE, SIG_IGN);
356+
/* Closing the pipe signals the subprocess to initiate a shutdown. */
357+
close(process->in);
358+
close(process->out);
359+
sigchain_pop(SIGPIPE);
360+
/*
361+
* In contrast to subprocess_exit_handler(), do _not_ wait for the
362+
* process to finish on its own accord: It needs to be terminated via
363+
* a signal, which is what `cleanup_children()` will do after this
364+
* function returns.
365+
*/
366+
}
367+
338368
static struct gh_server__process *gh_client__find_long_running_process(
339369
unsigned int cap_needed)
340370
{
@@ -386,6 +416,9 @@ static struct gh_server__process *gh_client__find_long_running_process(
386416
&entry->subprocess, 1,
387417
&argv, gh_client__start_fn))
388418
FREE_AND_NULL(entry);
419+
else
420+
entry->subprocess.process.clean_on_exit_handler =
421+
gh_client__subprocess_exit_handler;
389422
}
390423

391424
if (entry &&

t/t9210-scalar.sh

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,10 @@ test_expect_success '`scalar clone` with GVFS-enabled server' '
426426
)
427427
'
428428

429+
test_expect_success 'fetch <non-existent> does not hang in gvfs-helper' '
430+
test_must_fail git -C using-gvfs/src fetch origin does-not-exist
431+
'
432+
429433
test_expect_success '`scalar clone --no-gvfs-protocol` skips gvfs/config' '
430434
# the fake cache server requires fake authentication &&
431435
git config --global core.askPass true &&

0 commit comments

Comments
 (0)