freshlybakedca.ke / git
Git fork
fork atom

perl Git.pm: don't ignore signalled failure in _cmd_close()

Fix misbehavior in Git.pm that dates back to the very first version of
the library in git.git added in b1edc53d062 (Introduce Git.pm (v4),
2006-06-24). When we fail to execute a command we shouldn't ignore all
signals, those can happen e.g. if abort() is called, or if the command
segfaults.

Because of this we'd consider e.g. a command that died due to LSAN
exiting with abort() successful, as is the case with the tests listed
as running successfully with SANITIZE=leak in 9081a421a6d (checkout:
fix "branch info" memory leaks, 2021-11-16). We did run them
successfully, but only because we ignored these errors.

This was then made worse by the use of "abort_on_error=1" for LSAN
added in 85b81b35ff9 (test-lib: set LSAN_OPTIONS to abort by default,
2017-09-05). Doing that makes sense, but without providing that option
we'd have a "$? >> 8" of "23" on failure, with abort_on_error=1 we'll
get "0".

All of our tests pass even without the SIGPIPE exception being added
here, but as the code appears to have been trying to ignore it let's
keep ignoring it for now.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

authored by

Ævar Arnfjörð Bjarmason and committed by
Junio C Hamano 6798b08e 4c53a8c2

+19 -6
unified split
+19 -2
perl/Git.pm
··· 1686 1686 # by searching for it at proper places. 1687 1687 sub _execv_git_cmd { exec('git', @_); } 1688 1688 1689 + sub _is_sig { 1690 + my ($v, $n) = @_; 1691 + 1692 + # We are avoiding a "use POSIX qw(SIGPIPE SIGABRT)" in the hot 1693 + # Git.pm codepath. 1694 + require POSIX; 1695 + no strict 'refs'; 1696 + $v == *{"POSIX::$n"}->(); 1697 + } 1698 + 1689 1699 # Close pipe to a subprocess. 1690 1700 sub _cmd_close { 1691 1701 my $ctx = shift @_; ··· 1698 1708 } elsif ($? >> 8) { 1699 1709 # The caller should pepper this. 1700 1710 throw Git::Error::Command($ctx, $? >> 8); 1711 + } elsif ($? & 127 && _is_sig($? & 127, "SIGPIPE")) { 1712 + # we might e.g. closed a live stream; the command 1713 + # dying of SIGPIPE would drive us here. 1714 + } elsif ($? & 127 && _is_sig($? & 127, "SIGABRT")) { 1715 + die sprintf('BUG?: got SIGABRT ($? = %d, $? & 127 = %d) when closing pipe', 1716 + $?, $? & 127); 1717 + } elsif ($? & 127) { 1718 + die sprintf('got signal ($? = %d, $? & 127 = %d) when closing pipe', 1719 + $?, $? & 127); 1701 1720 } 1702 - # else we might e.g. closed a live stream; the command 1703 - # dying of SIGPIPE would drive us here. 1704 1721 } 1705 1722 } 1706 1723
-1
t/t9102-git-svn-deep-rmdir.sh
··· 1 1 #!/bin/sh 2 2 test_description='git svn rmdir' 3 3 4 - TEST_PASSES_SANITIZE_LEAK=true 5 4 . ./lib-git-svn.sh 6 5 7 6 test_expect_success 'initialize repo' '
-1
t/t9123-git-svn-rebuild-with-rewriteroot.sh
··· 5 5 6 6 test_description='git svn respects rewriteRoot during rebuild' 7 7 8 - TEST_PASSES_SANITIZE_LEAK=true 9 8 . ./lib-git-svn.sh 10 9 11 10 mkdir import
-1
t/t9128-git-svn-cmd-branch.sh
··· 5 5 6 6 test_description='git svn partial-rebuild tests' 7 7 8 - TEST_PASSES_SANITIZE_LEAK=true 9 8 . ./lib-git-svn.sh 10 9 11 10 test_expect_success 'initialize svnrepo' '
-1
t/t9167-git-svn-cmd-branch-subproject.sh
··· 5 5 6 6 test_description='git svn branch for subproject clones' 7 7 8 - TEST_PASSES_SANITIZE_LEAK=true 9 8 . ./lib-git-svn.sh 10 9 11 10 test_expect_success 'initialize svnrepo' '