# [PATCH] enable "no-done" extension only when fetching over smart-http 5 messages from 2011-03-29 to 2011-03-29. Participants: Junio C Hamano, Shawn Pearce. Thread: https://gitlist.dev/t/26912 ## Junio C Hamano, 2011-03-29 17:16 Subject: [PATCH] enable "no-done" extension only when fetching over smart-http Message-ID: <7vlizxsv82.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vlizxsv82.fsf%40alter.siamese.dyndns.org ``` 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 --- builtin/fetch-pack.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) 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, 2011-03-29 17:24 Subject: Re: [PATCH] enable "no-done" extension only when fetching over smart-http Message-ID: URL: https://gitlist.dev/e/AANLkTimg5_9dQDq8Xa9pJsx9f6UrNewih%2BWirKcOP42i%40mail.gmail.com In-Reply-To: <7vlizxsv82.fsf@alter.siamese.dyndns.org> ``` On Tue, Mar 29, 2011 at 10:16, Junio C Hamano wrote: > 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 Acked-by: Shawn O. Pearce -- Shawn. ``` ## Junio C Hamano, 2011-03-29 17:24 Subject: [PATCH] enable "no-done" extension only when serving over smart-http Message-ID: <7vhbalsutw.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vhbalsutw.fsf%40alter.siamese.dyndns.org In-Reply-To: <7vlizxsv82.fsf@alter.siamese.dyndns.org> ``` 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 --- upload-pack.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) 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, 2011-03-29 17:31 Subject: Re: [PATCH] enable "no-done" extension only when serving over smart-http Message-ID: URL: https://gitlist.dev/e/AANLkTinuWGN9FcQoONboFK9WdrRYU%2BNypmWjNAfGc%3D_B%40mail.gmail.com In-Reply-To: <7vhbalsutw.fsf@alter.siamese.dyndns.org> ``` On Tue, Mar 29, 2011 at 10:24, Junio C Hamano wrote: > 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 Acked-by: Shawn O. Pearce -- Shawn. ``` ## Junio C Hamano, 2011-03-29 17:32 Subject: [PATCH] enable "no-done" extension only when serving over smart-http Message-ID: <7vfwq5suhp.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vfwq5suhp.fsf%40alter.siamese.dyndns.org In-Reply-To: <7vlizxsv82.fsf@alter.siamese.dyndns.org> ``` 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 --- * Sorry for the resend... upload-pack.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) 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; ```