Thread (6 messages) 6 messages, 3 authors, 2016-06-15

[PATCH] Fix potential local deadlock during fetch-pack

flat view
STALE3776d

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:50:56
Subsystem: the rest · Maintainer: Linus Torvalds

The fetch-pack/upload-pack protocol relies on the underlying transport
(local pipe or TCP socket) to have enough slack to allow one window worth
of data in flight without blocking the writer.  Traditionally we always
relied on being able to have a batch of 32 "have"s in flight (roughly 1.5k
bytes) to stream.

The recent "progressive-stride" change allows "fetch-pack" to send up to
1024 "have"s without reading any response from "upload-pack".  The
outgoing pipe of "upload-pack" can be clogged with many ACK and NAK that
are unread, while "fetch-pack" is still stuffing its outgoing pike with
more "have"s, leading to a deadlock.

Revert the change unless we are in stateless rpc (aka smart-http) mode, as
using a large window full of "have"s is still a good way to help reduce
the number of back-and-forth, and there is no buffering issue there (it is
strictly "ping-pong" without an overlap).

Signed-off-by: Junio C Hamano <redacted>
---
 builtin/fetch-pack.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 1724b76..e5fdacd 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -229,16 +229,17 @@ static void insert_alternate_refs(void)
 }
 
 #define INITIAL_FLUSH 16
+#define PIPESAFE_FLUSH 32
 #define LARGE_FLUSH 1024
 
 static int next_flush(int count)
 {
-	if (count < INITIAL_FLUSH * 2)
-		count += INITIAL_FLUSH;
-	else if (count < LARGE_FLUSH)
+	int flush_limit = args.stateless_rpc ? LARGE_FLUSH : PIPESAFE_FLUSH;
+
+	if (count < flush_limit)
 		count <<= 1;
 	else
-		count += LARGE_FLUSH;
+		count += flush_limit;
 	return count;
 }
 
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help