Skip to content

Commit

Permalink
Fix potential local deadlock during fetch-pack
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Junio C Hamano committed Mar 29, 2011
1 parent 066bf4c commit 44d8dc5
Showing 1 changed file with 5 additions and 4 deletions.
9 changes: 5 additions & 4 deletions builtin/fetch-pack.c
Original file line number Diff line number Diff line change
Expand Up @@ -219,16 +219,17 @@ static void send_request(int fd, struct strbuf *buf)
}

#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;
}

Expand Down

0 comments on commit 44d8dc5

Please sign in to comment.