Skip to content

Commit 5bafc74

Browse files
derrickstoleedscho
andcommitted
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 <stuartwilcox@microsoft.com> Co-authored-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
1 parent bd63763 commit 5bafc74

File tree

2 files changed

+30
-0
lines changed

2 files changed

+30
-0
lines changed

gvfs-helper-client.c

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,29 @@ 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+
* Sadly, there is no way to pass the `struct subprocess_entry` instance to
348+
* the `clean_on_exit_handler`, it will only receive the `struct child_process`
349+
* pointer. But since we know that that pointer points to a field of
350+
* the `subprocess_entry` struct, we can use `container_of()` to get that
351+
* instance and then use it to stop the subprocess as desired.
352+
*/
353+
static void gh_client__subprocess_exit_handler(struct child_process *process)
354+
{
355+
struct gh_server__process *entry =
356+
container_of(process, struct gh_server__process, subprocess.process);
357+
358+
subprocess_stop(&gh_server__subprocess_map, &entry->subprocess);
359+
}
360+
338361
static struct gh_server__process *gh_client__find_long_running_process(
339362
unsigned int cap_needed)
340363
{
@@ -386,6 +409,9 @@ static struct gh_server__process *gh_client__find_long_running_process(
386409
&entry->subprocess, 1,
387410
&argv, gh_client__start_fn))
388411
FREE_AND_NULL(entry);
412+
else
413+
entry->subprocess.process.clean_on_exit_handler =
414+
gh_client__subprocess_exit_handler;
389415
}
390416

391417
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)