threads / patch / 26912

patchenable "no-done" extension only when fetching over smart-http

Subject: [PATCH] enable "no-done" extension only when fetching over smart-http

## tl;dr

5 messages between Mar 29, 2011 and Mar 29, 2011. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Junio C Hamano· Mar 29, 2011, 17:16 UTC · lore

When 'no-done' protocol extension is used, the upload-pack (i.e. the server side) process stops listening to the fetch-pack after issuing the final NAK, and starts sending the generated pack data back, but there may be more "have" send by the latter in flight that the fetch-pack is expecting to be responded with ACK/NAK. This will typically result in a deadlock (both will block on write that the other end never reads) or SIGPIPE on the fetch-pack end (upload-pack will finish writing a small pack and goes away).

Disable it unless fetch-pack is running under smart-http, where there is no such streaming issue.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/fetch-pack.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

Show changes to builtin/fetch-pack.c +2 −1

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 59fbda5..52707a8 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -708,7 +708,8 @@ static struct ref *do_fetch_pack(int fd[2],
 		if (server_supports("no-done")) {
 			if (args.verbose)
 				fprintf(stderr, "Server supports no-done\n");
-			no_done = 1;
+			if (args.stateless_rpc)
+				no_done = 1;
 		}
 	}
 	else if (server_supports("multi_ack")) {

Shawn Pearce· Mar 29, 2011, 17:24 UTC · re: Junio C Hamano · lore

Re: [PATCH] enable "no-done" extension only when fetching over smart-http

On Tue, Mar 29, 2011 at 10:16, Junio C Hamano <gitster@pobox.com> wrote:

Show 13 quoted lines

> When 'no-done' protocol extension is used, the upload-pack (i.e. the
> server side) process stops listening to the fetch-pack after issuing the
> final NAK, and starts sending the generated pack data back, but there may
> be more "have" send by the latter in flight that the fetch-pack is
> expecting to be responded with ACK/NAK.  This will typically result in a
> deadlock (both will block on write that the other end never reads) or
> SIGPIPE on the fetch-pack end (upload-pack will finish writing a small
> pack and goes away).
>
> Disable it unless fetch-pack is running under smart-http, where there is
> no such streaming issue.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Acked-by: Shawn O. Pearce <spearce@spearce.org>

-- 
Shawn.

Junio C Hamano· Mar 29, 2011, 17:24 UTC · re: Junio C Hamano · lore

[PATCH] enable "no-done" extension only when serving over smart-http

Do not advertise no-done capability when upload-pack is not serving over smart-http, as there is no way for this server to know when it should stop reading in-flight data from the client, even though it is necessary to drain all the in-flight data in order to unblock the client.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 upload-pack.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

Show changes to upload-pack.c +4 −3

diff --git a/upload-pack.c b/upload-pack.c
index 5924f6f..a247fb9 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -640,15 +640,16 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo
 {
 	static const char *capabilities = "multi_ack thin-pack side-band"
 		" side-band-64k ofs-delta shallow no-progress"
-		" include-tag multi_ack_detailed no-done";
+		" include-tag multi_ack_detailed";
 	struct object *o = parse_object(sha1);
 
 	if (!o)
 		die("git upload-pack: cannot find object %s:", sha1_to_hex(sha1));
 
 	if (capabilities)
-		packet_write(1, "%s %s%c%s\n", sha1_to_hex(sha1), refname,
-			0, capabilities);
+		packet_write(1, "%s %s%c%s%s\n", sha1_to_hex(sha1), refname,
+			     0, capabilities,
+			     stateless_rpc ? " no-done" : "");
 	else
 		packet_write(1, "%s %s\n", sha1_to_hex(sha1), refname);
 	capabilities = NULL;

Shawn Pearce· Mar 29, 2011, 17:31 UTC · re: Junio C Hamano · lore

Re: [PATCH] enable "no-done" extension only when serving over smart-http

On Tue, Mar 29, 2011 at 10:24, Junio C Hamano <gitster@pobox.com> wrote:

Show 6 quoted lines

> Do not advertise no-done capability when upload-pack is not serving over
> smart-http, as there is no way for this server to know when it should stop
> reading in-flight data from the client, even though it is necessary to
> drain all the in-flight data in order to unblock the client.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Acked-by: Shawn O. Pearce <spearce@spearce.org>

-- 
Shawn.

Junio C Hamano· Mar 29, 2011, 17:32 UTC · re: Junio C Hamano · lore

[PATCH] enable "no-done" extension only when serving over smart-http

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * Sorry for the resend...

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

Show changes to upload-pack.c +4 −3

diff --git a/upload-pack.c b/upload-pack.c
index 5924f6f..a247fb9 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -640,15 +640,16 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo
 {
 	static const char *capabilities = "multi_ack thin-pack side-band"
 		" side-band-64k ofs-delta shallow no-progress"
-		" include-tag multi_ack_detailed no-done";
+		" include-tag multi_ack_detailed";
 	struct object *o = parse_object(sha1);
 
 	if (!o)
 		die("git upload-pack: cannot find object %s:", sha1_to_hex(sha1));
 
 	if (capabilities)
-		packet_write(1, "%s %s%c%s\n", sha1_to_hex(sha1), refname,
-			0, capabilities);
+		packet_write(1, "%s %s%c%s%s\n", sha1_to_hex(sha1), refname,
+			     0, capabilities,
+			     stateless_rpc ? " no-done" : "");
 	else
 		packet_write(1, "%s %s\n", sha1_to_hex(sha1), refname);
 	capabilities = NULL;