[PATCH v2 1/5] remote-curl: avoid hang if curl asks for more data after eof
To
git@vger.kernel.org
Cc
Jeff King
Jonathan Tan
Junio C Hamano
From
Jiří Hruška
See Also
Ref 1
Date
2023-11-15 03:34:51 UTC
It has been observed that under some circumstances, libcurl can call
our `CURLOPT_READFUNCTION` callback `rpc_out()` again even after
already getting a return value of zero (EOF) back once before.

Because `rpc->flush_read_but_not_sent` is reset to false immediately
the first time an EOF is returned, the repeated call goes again to
`rpc_read_from_out()`, which tries to read more from the child process
pipe and the whole operation gets stuck - the child process is already
trying to read a response back and will not write anything to the
output pipe anymore, while the parent/remote process is now blocked
waiting to read more too and never even finishes sending the request.

The bug is fixed by moving the reset of the `flush_read_but_not_sent`
flag to `post_rpc()`, only before `rpc_out()` would be potentially used
the next time. This makes the callback behave like fread() and return
a zero any number of times at the end of a finished upload.

Signed-off-by: Jiri Hruska <jirka@fud.cz>
---
 remote-curl.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/remote-curl.c b/remote-curl.c
index ef05752ca5..199c4615a5 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -705,9 +705,10 @@ static size_t rpc_out(void *ptr, size_t eltsize,
 			 * The line length either does not need to be sent at
 			 * all or has already been completely sent. Now we can
 			 * return 0, indicating EOF, meaning that the flush has
-			 * been fully sent.
+			 * been fully sent. It is important to keep returning 0
+			 * as long as needed in that case, as libcurl invokes
+			 * the callback multiple times at EOF sometimes.
 			 */
-			rpc->flush_read_but_not_sent = 0;
 			return 0;
 		}
 		/*
@@ -963,6 +964,7 @@ static int post_rpc(struct rpc_state *rpc, int
stateless_connect, int flush_rece
 		 */
 		headers = curl_slist_append(headers, "Transfer-Encoding: chunked");
 		rpc->initial_buffer = 1;
+		rpc->flush_read_but_not_sent = 0;
 		curl_easy_setopt(slot->curl, CURLOPT_READFUNCTION, rpc_out);
 		curl_easy_setopt(slot->curl, CURLOPT_INFILE, rpc);
 		curl_easy_setopt(slot->curl, CURLOPT_SEEKFUNCTION, rpc_seek);
-- 
2.42.1.5.g2f21867bd5