summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJeff King <peff@peff.net>2012-02-23 05:04:34 -0500
committerJunio C Hamano <gitster@pobox.com>2012-02-23 11:10:10 -0800
commitc34fe6304c0162c526b74e8639c94d8d026615fd (patch)
tree26dfc50b99bc6c82abd7c3ecaa9df38bb82bf519
parent5238cbf65638d7a097bdb5ca8226f5acbe31f143 (diff)
downloadgit-c34fe6304c0162c526b74e8639c94d8d026615fd.tar.gz
disconnect from remote helpers more gentlysp/smart-http-failure-to-push
When git spawns a remote helper program (like git-remote-http), the last thing we do before closing the pipe to the child process is to send a blank line, telling the helper that we are done issuing commands. However, the helper may already have exited, in which case the parent git process will receive SIGPIPE and die. In particular, this can happen with the remote-curl helper when it encounters errors during a push. The helper reports individual errors for each ref back to git-push, and then exits with a non-zero exit code. Depending on the exact timing of the write, the parent process may or may not receive SIGPIPE. This causes intermittent test failure in t5541.8, and is a side effect of 5238cbf (remote-curl: Fix push status report when all branches fail). Before that commit, remote-curl would not send the final blank line to indicate that the list of status lines was complete; it would just exit, closing the pipe. The parent git-push would notice the closed pipe while reading the status report and exit immediately itself, propagating the failing exit code. But post-5238cbf, remote-curl completes the status list before exiting, git-push actually runs to completion, and then it tries to cleanly disconnect the helper, leading to the SIGPIPE race above. This patch drops all error-checking when sending the final "we are about to hang up" blank line to helpers. There is nothing useful for the parent process to do about errors at that point anyway, and certainly failing to send our "we are done with commands" line to a helper that has already exited is not a problem. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
-rw-r--r--transport-helper.c13
1 files changed, 10 insertions, 3 deletions
diff --git a/transport-helper.c b/transport-helper.c
index 660147f9e9..7636a2771a 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -9,6 +9,7 @@
#include "remote.h"
#include "string-list.h"
#include "thread-utils.h"
+#include "sigchain.h"
static int debug;
@@ -203,14 +204,20 @@ static struct child_process *get_helper(struct transport *transport)
static int disconnect_helper(struct transport *transport)
{
struct helper_data *data = transport->data;
- struct strbuf buf = STRBUF_INIT;
if (data->helper) {
if (debug)
fprintf(stderr, "Debug: Disconnecting.\n");
if (!data->no_disconnect_req) {
- strbuf_addf(&buf, "\n");
- sendline(data, &buf);
+ /*
+ * Ignore write errors; there's nothing we can do,
+ * since we're about to close the pipe anyway. And the
+ * most likely error is EPIPE due to the helper dying
+ * to report an error itself.
+ */
+ sigchain_push(SIGPIPE, SIG_IGN);
+ xwrite(data->helper->in, "\n", 1);
+ sigchain_pop(SIGPIPE);
}
close(data->helper->in);
close(data->helper->out);