summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJeff King <peff@peff.net>2019-03-03 16:58:43 (GMT)
committerJunio C Hamano <gitster@pobox.com>2019-03-05 06:02:18 (GMT)
commit143588949c8a0672302403c722fc433a5bb2ea2a (patch)
tree1a7a06cfbe6bf8e049530bcbdb560cff9838f775
parent37c80012f1415bc4ff23def735a87cbd395bca31 (diff)
downloadgit-143588949c8a0672302403c722fc433a5bb2ea2a.zip
git-143588949c8a0672302403c722fc433a5bb2ea2a.tar.gz
git-143588949c8a0672302403c722fc433a5bb2ea2a.tar.bz2
fetch: ignore SIGPIPE during network operation
The default SIGPIPE behavior can be useful for a command that generates a lot of output: if the receiver of our output goes away, we'll be notified asynchronously to stop generating it (typically by killing the program). But for a command like fetch, which is primarily concerned with receiving data and writing it to disk, an unexpected SIGPIPE can be awkward. We're already checking the return value of all of our write() calls, and dying due to the signal takes away our chance to gracefully handle the error. On Linux, we wouldn't generally see SIGPIPE at all during fetch. If the other side of the network connection hangs up, we'll see ECONNRESET. But on OS X, we get a SIGPIPE, and the process is killed. This causes t5570 to racily fail, as we sometimes die by signal (instead of the expected die() call) when the server side hangs up. Let's ignore SIGPIPE during the network portion of the fetch, which will cause our write() to return EPIPE, giving us consistent behavior across platforms. This fixes the test flakiness, but note that it stops short of fixing the larger problem. The server side hit a fatal error, sent us an "ERR" packet, and then hung up. We notice the failure because we're trying to write to a closed socket. But by dying immediately, we never actually read the ERR packet and report its content to the user. This is a (racy) problem on all platforms. So this patch lays the groundwork from which that problem might be fixed consistently, but it doesn't actually fix it. Note the placement of the SIGPIPE handling. The absolute minimal change would be to ignore SIGPIPE only when we're writing. But twiddling the signal handler for each write call is inefficient and maintenance burden. On the opposite end of the spectrum, we could simply declare that fetch does not need SIGPIPE handling, since it doesn't generate a lot of output, and we could just ignore it at the start of cmd_fetch(). This patch takes a middle ground. It ignores SIGPIPE during the network operation (which is admittedly most of the program, since the actual network operations are all done under the hood by the transport code). So it's still pretty coarse. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
-rw-r--r--builtin/fetch.c2
1 files changed, 2 insertions, 0 deletions
diff --git a/builtin/fetch.c b/builtin/fetch.c
index e014032..d10a67c 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1554,7 +1554,9 @@ static int fetch_one(struct remote *remote, int argc, const char **argv, int pru
sigchain_push_common(unlock_pack_on_signal);
atexit(unlock_pack);
+ sigchain_push(SIGPIPE, SIG_IGN);
exit_code = do_fetch(gtransport, &rs);
+ sigchain_pop(SIGPIPE);
refspec_clear(&rs);
transport_disconnect(gtransport);
gtransport = NULL;