threads / patch / 18326

patchjgit: upload-pack: Force an fd flush after receiving flush pkt from client

Subject: [PATCH] jgit: upload-pack: Force an fd flush after receiving flush pkt from client

## tl;dr

2 messages between Mar 16, 2009 and Mar 16, 2009. Diffs are folded; open one to read it.

replies: 1people: 2as markdown or json

Dima Zavin· Mar 16, 2009, 17:34 UTC · lore
Signed-off-by: Dima Zavin <dima@android.com>
---
 .../src/org/spearce/jgit/transport/UploadPack.java |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)
Show changes to org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java +1 −2
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java
index 71acda1..80c154b 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java
@@ -351,7 +351,7 @@ private void negotiate() throws IOException {
 			if (line.length() == 0) {
 				if (commonBase.isEmpty() || multiAck)
 					pckOut.writeString("NAK\n");
-
+				pckOut.flush();
 			} else if (line.startsWith("have ") && line.length() == 45) {
 				final ObjectId id = ObjectId.fromString(line.substring(5));
 				if (matchHave(id)) {
@@ -372,7 +372,6 @@ private void negotiate() throws IOException {
 			} else if (line.equals("done")) {
 				if (commonBase.isEmpty())
 					pckOut.writeString("NAK\n");
-
 				else if (multiAck)
 					pckOut.writeString("ACK " + last.name() + "\n");
 				break;
-- 
1.6.2
Shawn O. Pearce· Mar 16, 2009, 20:05 UTC · re: Dima Zavin · lore

Re: [PATCH] jgit: upload-pack: Force an fd flush after receiving flush pkt from client

Dima Zavin <dima@android.com> wrote:
Show 13 quoted lines
> diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java
> index 71acda1..80c154b 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/transport/UploadPack.java
> @@ -351,7 +351,7 @@ private void negotiate() throws IOException {
>  			if (line.length() == 0) {
>  				if (commonBase.isEmpty() || multiAck)
>  					pckOut.writeString("NAK\n");
> -
> +				pckOut.flush();
>  			} else if (line.startsWith("have ") && line.length() == 45) {
>  				final ObjectId id = ObjectId.fromString(line.substring(5));
>  				if (matchHave(id)) {
Applied, but with this more verbose commit message:

--8<-- upload-pack: Force an fd flush after receiving flush pkt from client

The client is blocked waiting for an ACK or NAK line from the server. If we don't call flush() here there is a very good chance the ACK/NAK is stuck in our stream buffer within the JRE, and doesn't make it into the kernel's TCP buffer. This causes the server to wait for more have lines, and the client to wait for the ACK/NAK, and the entire thing just deadlocks.

We flush anytime we see a pkt-line flush command, as there may be buffered ACK lines from prior evaulations that need to be sent to the waiting client.

Signed-off-by: Dima Zavin <dima@android.com>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
-- 
Shawn.

← back to recent threads