builtin/gc: stop processing log file on signal
commit9b6b994f90f4427a0ddef38594036055e7b0efa1
authorPatrick Steinhardt <ps@pks.im>
Fri, 16 Aug 2024 10:45:06 +0000 (16 12:45 +0200)
committerJunio C Hamano <gitster@pobox.com>
Fri, 16 Aug 2024 16:46:25 +0000 (16 09:46 -0700)
treefd23f60eafe84536e7bfc5b70e802794b6005c4d
parent0ce44e2293352bc3636d93c156fbc2a1ebac0e45
builtin/gc: stop processing log file on signal

When detaching, git-gc(1) will redirect its stderr to a "gc.log" log
file, which is then used to surface errors of a backgrounded process to
the user. To ensure that the file is properly managed on abnormal exit
paths, we install both signal and exit handlers that try to either
commit the underlying lock file or roll it back in case there wasn't any
error.

This logic is severly broken when handling signals though, as we end up
calling all kinds of functions that are not signal safe. This includes
malloc(3P) via `git_path()`, fprintf(3P), fflush(3P) and many more
functions. The consequence can be anything, from deadlocks to crashes.
Unfortunately, we cannot really do much about this without a larger
refactoring.

The least-worst thing we can do is to not set up the signal handler in
the first place. This will still cause us to remove the lockfile, as the
underlying tempfile subsystem already knows to unlink locks when
receiving a signal. But it may cause us to remove the lock even in the
case where it would have contained actual errors, which is a change in
behaviour.

The consequence is that "gc.log" will not be committed, and thus
subsequent calls to `git gc --auto` won't bail out because of this.
Arguably though, it is better to retry garbage collection rather than
having the process run into a potentially-corrupted state.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin/gc.c