@matheusbsbs / matheus-bagliano / commits / 44d8dc54e7
Fork 0

Fix potential local deadlock during fetch-pack

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 two windows of 32 "have"s in flight (roughly 3k 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 pipe 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 <gitster@pobox.com>

Junio C Hamano committed Mar 29, 2011 at 10:06 UTC 44d8dc54e73e8010c4bdf57a422fc8d5ce709029
1 file changed +5 -4
builtin/fetch-pack.c
+5 -4
@@ -219,16 +219,17 @@ static void send_request(int fd, struct strbuf *buf)
219 }
220
221 #define INITIAL_FLUSH 16
222 +#define PIPESAFE_FLUSH 32
223 #define LARGE_FLUSH 1024
224
225 static int next_flush(int count)
226 {
226 - if (count < INITIAL_FLUSH * 2)
227 - count += INITIAL_FLUSH;
228 - else if (count < LARGE_FLUSH)
227 + int flush_limit = args.stateless_rpc ? LARGE_FLUSH : PIPESAFE_FLUSH;
228 +
229 + if (count < flush_limit)
230 count <<= 1;
231 else
231 - count += LARGE_FLUSH;
232 + count += flush_limit;
233 return count;
234 }
235