git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Sixt <j.sixt@viscovery.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: sam@vilain.net, git@vger.kernel.org,
	Nick Edelen <sirnot@gmail.com>,
	"Shawn O. Pearce" <spearce@spearce.org>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Andreas Ericsson <exon@op5.se>,
	Christian Couder <christian@couder.net>,
	Jeff King <peff@peff.net>
Subject: [PATCH] fetch-pack: close output channel after sideband demultiplexer terminates
Date: Mon, 08 Jun 2009 10:51:22 +0200	[thread overview]
Message-ID: <4A2CD10A.3000703@viscovery.net> (raw)
In-Reply-To: <4A28D2EF.8040704@viscovery.net>

From: Johannes Sixt <j6t@kdbg.org>

fetch-pack runs the sideband demultiplexer using start_async(). This
facility requires that the asynchronously executed function closes the
output file descriptor (see Documentation/technical/api-run-command.txt).
But the sideband demultiplexer did not do that. This fixes it.

In certain error situations this could lock up a fetch operation on
Windows because the asynchronous function is run in a thread; by not
closing the output fd the reading end never got EOF and waited for more
data indefinitely. On Unix this is not a problem because the asynchronous
function is run in a separate process, which exits after the function ends
and so implicitly closes the output.

Since the pack that is sent over the wire encodes the number of objects in
the stream, during normal operation the reading end knows when the stream
ends and terminates by itself, and does not lock up.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---

 Johannes Sixt schrieb:
 > FYI, with this patch MinGW git hangs in t5530.8; the test-case exercises
 > the new code path.

 This fixes the lockup.

 -- Hannes

 builtin-fetch-pack.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index 2eb7a22..b11d799 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -487,7 +487,9 @@ static int sideband_demux(int fd, void *data)
 {
 	int *xd = data;

-	return recv_sideband("fetch-pack", xd[0], fd);
+	int ret = recv_sideband("fetch-pack", xd[0], fd);
+	close(fd);
+	return ret;
 }

 static int get_pack(int xd[2], char **pack_lockfile)
-- 
1.6.3.1.1141.gcf6b0.dirty

  reply	other threads:[~2009-06-08  8:51 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-06-05  5:45 [WIP] Shift rev-list enumeration from upload-pack to pack-objects sam, Nick Edelen
2009-06-05  5:46 ` Sam Vilain
2009-06-05  8:10 ` Johannes Sixt
2009-06-08  8:51   ` Johannes Sixt [this message]
2009-06-05 16:51 ` Nicolas Pitre
2009-06-07 13:25   ` Nick Edelen
2009-06-07 13:31     ` Nick Edelen
2009-06-07 16:41     ` Nicolas Pitre
2009-06-07 16:47       ` Nick Edelen
2009-06-07 18:55         ` Nick Edelen
2009-06-07 20:48           ` Sam Vilain
2009-06-07 20:48           ` Nicolas Pitre
2009-06-07 22:04             ` Nick Edelen
2009-06-08  0:50               ` Nicolas Pitre
2009-06-08  2:27                 ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4A2CD10A.3000703@viscovery.net \
    --to=j.sixt@viscovery.net \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=christian@couder.net \
    --cc=exon@op5.se \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=sam@vilain.net \
    --cc=sirnot@gmail.com \
    --cc=spearce@spearce.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).