{"thread":{"id":"44750","subject":"Bug report: Git pull hang occasionally","startedAt":"2016-12-21T19:48:20Z","lastAt":"2017-01-12T21:17:38Z","messageCount":8,"participants":["Kai Zhang","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"308212","messageId":"9B7DCFB3-73A4-40DE-8FC6-867C5016EF95@netskope.com","threadId":"44750","inReplyTo":null,"subject":"Bug report: Git pull hang occasionally","fromName":"Kai Zhang","fromEmail":"kai@netskope.com","sentAt":"2016-12-21T19:47:56Z","receivedAt":"2016-12-21T19:48:20Z","isPatch":false,"sender":{"key":"kai@netskope.com","avatar":null},"body":"Issue: Git pull hang occasionally, and when git pull start hanging, need manually \"kill -9\" to stop hanging\n\nEnvironment:\nServer side:\nGit version: 2.11.0\nOS: ubuntu 12.04\nNginx: 1.9.7.4\nfcgiwrap: 1.1.0\nGit repo: None bare, small size (less than 5 MB including .git folder), small file number (less than 100 files)\n\nNginx config for git:\n\n    location ~* /git(\\/.*) {\n        root /var/git;\n        fastcgi_buffers 256 8k;\n        fastcgi_param SCRIPT_FILENAME   /usr/lib/git-core/git-http-backend;\n        fastcgi_param GIT_HTTP_EXPORT_ALL       true;\n        fastcgi_param GIT_PROJECT_ROOT          /var/git;\n        fastcgi_param PATH_INFO                 $1;\n        fastcgi_pass unix:/var/run/fcgiwrap.socket;\n        include         /opt/openresty/nginx/conf/fastcgi_params;\n    }\n\nClient side:\nGit version: 2.11.0\nOS: ubuntu 12.04\nAll git operations go through http only\n\nEnd to end work flow:\nKeep committing small files to non-bare git repo on server side (twice per second), message will be sent to client side for every update, once client receives message, do git pull\n\nHanging frequency:\nAround 4 times a day\n\nHanging command stack:\nroot     32640 23228  0 20:51 ?        00:00:00 git pull -v remote_name master --allow-unrelated-histories\nroot     32641 32640  0 20:51 ?        00:00:00 git fetch --update-head-ok -v remote_name master\nroot     32642 32641  0 20:51 ?        00:00:00 git-remote-http remote_name http://server:80/git/repo_name/.git\nroot     32651 32642  0 20:51 ?        00:00:00 git fetch-pack --stateless-rpc --stdin --lock-pack --thin --no-progress http://server:80/git/repo_name/.git/\n\nAccess log for hanging git pull:\n10.1.0.10 - - [20/Dec/2016:20:38:10 +0000] \"GET /git/repo_name/.git/info/refs?service=git-upload-pack HTTP/1.1\" 200 363 \"-\" \"git/2.11.0\" \"-\"\n10.1.0.10 - - [20/Dec/2016:20:38:10 +0000] \"POST /git/repo_name/.git/git-upload-pack HTTP/1.1\" 200 5 \"-\" \"git/2.11.0\" \"-\"\n\nError log for hanging git pull:\n2016/12/20 20:38:10 [error] 9957#0: *687703 FastCGI sent in stderr: \"fatal: 'HEAD' is a symref but it is not?\" while reading response header from upstream, client: 10.1.0.11, server: server, request: \"POST /git/repo_name/.git/git-upload-pack HTTP/1.1\", upstream: \"fastcgi://unix:/var/run/fcgiwrap.socket:\", host: \"server\"\n\n\nSome observation:\n1. When hanging happen, same repository could be cloned or pulled by another process on the same client.\n2. After killing hanging git pull, during retry,  same repository can be sync up successfully.\n3. Git pull has been executed twice per second. But hanging only happens around 4 times a day.\n4. When \"fatal: 'HEAD' is a symref but it is not?\" happen for POST on server side, client side always start to hang. And when hanging happen on client side, this log for POST always appears. But, if  \"fatal: 'HEAD' is a symref but it is not?\" happen for GET request on server side, client side never hang. For example: \n\n2016/12/20 20:36:53 [error] 9954#0: *685174 FastCGI sent in stderr: \"fatal: 'HEAD' is a symref but it is not?\" while reading response header from upstream, client: 10.1.0.11, server: server, request: \"GET /git/repo_name/.git/info/refs?service=git-upload-pack HTTP/1.1\", upstream: \"fastcgi://unix:/var/run/fcgiwrap.socket:\", host: \"server\"\n\nwill not trigger hanging on client side. And this log \"fatal: 'HEAD' is a symref but it is not?\" is happening very rare (less than 10 times a day).\n\n\nIt seems a error handling issue on client side. Any help or pointer on where to look will be appreciated.\n\nRegards\nKai\n\nPS. I am not subscribed to the mailing list, please keep me in Cc\n\n\n"},{"id":"308215","messageId":"xmqq8tr9huc0.fsf@gitster.mtv.corp.google.com","threadId":"44750","inReplyTo":"9B7DCFB3-73A4-40DE-8FC6-867C5016EF95@netskope.com","subject":"Re: Bug report: Git pull hang occasionally","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-21T20:59:27Z","receivedAt":"2016-12-21T20:59:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kai Zhang <kai@netskope.com> writes:\n\n> 2016/12/20 20:38:10 [error] 9957#0: *687703 FastCGI sent in stderr: \"fatal: 'HEAD' is a symref but it is not?\" while reading response header from upstream, client: 10.1.0.11, server: server, request: \"POST /git/repo_name/.git/git-upload-pack HTTP/1.1\", upstream: \"fastcgi://unix:/var/run/fcgiwrap.socket:\", host: \"server\"\n\n(Not a solution)\n\nIn order to tell the client if HEAD is a symbolic ref and to what\nunderlying ref it points at if it is a symbolic ref, at the very\nbeginning of upload-pack, there is a call to head_ref_namespaced()\nthat uses find_symref().  find_symref() gets \"HEAD\" and a boolean\nthat says if it is a symbolic ref, but it does not get where the\nsymbolic ref points at, so it does resolve_ref_unsafe() to learn\nthat information.\n\nBetween the time head_ref_namespaced() checks the refs database and\nfinds that HEAD is a symbolic ref, and the time find_symref() calls\nresolve_ref_unsafe() to find out where it leads to, if somebody else\nupdates HEAD, resolve_ref_unsafe() can give an unexpected result, as\nall of these read-only operations are performed without any locking.\n\nAnd the unexpected discrepancy is reported by find_symref() as\nfatal.  The server side dies, and somehow that fact is lost between\nthe upload-pack process and the client and somebody in the middle\n(e.g. fastcgi interface or nginx webserver on the server side, or\nthe remote-curl helper on the client side) keeps the \"git fetch\"\nprocess waiting.\n\nSo there seem to be two issues.  \n\n - Because of the unlocked read, find_symref() can observe an\n   inconsistent state.  Perhaps it should be updated not to die but\n   to retry, expecting that transient inconsistency will go away.\n\n - A fatal error in upload-pack is not reported back to the client\n   to cause it exit is an obvious one, and even if we find a way to\n   make this fatal error in find_symref() not to trigger, fatal\n   errors in other places in the code can trigger the same symptom.\n\n"},{"id":"308218","messageId":"EB5CD780-BDDA-44C5-BD7D-1325F5891926@netskope.com","threadId":"44750","inReplyTo":"xmqq8tr9huc0.fsf@gitster.mtv.corp.google.com","subject":"Re: Bug report: Git pull hang occasionally","fromName":"Kai Zhang","fromEmail":"kai@netskope.com","sentAt":"2016-12-21T21:10:18Z","receivedAt":"2016-12-21T21:10:24Z","isPatch":false,"sender":{"key":"kai@netskope.com","avatar":null},"body":"Thank you for your insight and detailed explanation Junio.\n\nI think what you said is what is happening in my environment. Both writing and reading are happening simultaneously. \n\n\n> On Dec 21, 2016, at 12:59 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> \n> Kai Zhang <kai@netskope.com> writes:\n> \n>> 2016/12/20 20:38:10 [error] 9957#0: *687703 FastCGI sent in stderr: \"fatal: 'HEAD' is a symref but it is not?\" while reading response header from upstream, client: 10.1.0.11, server: server, request: \"POST /git/repo_name/.git/git-upload-pack HTTP/1.1\", upstream: \"fastcgi://unix:/var/run/fcgiwrap.socket:\", host: \"server\"\n> \n> (Not a solution)\n> \n> In order to tell the client if HEAD is a symbolic ref and to what\n> underlying ref it points at if it is a symbolic ref, at the very\n> beginning of upload-pack, there is a call to head_ref_namespaced()\n> that uses find_symref().  find_symref() gets \"HEAD\" and a boolean\n> that says if it is a symbolic ref, but it does not get where the\n> symbolic ref points at, so it does resolve_ref_unsafe() to learn\n> that information.\n> \n> Between the time head_ref_namespaced() checks the refs database and\n> finds that HEAD is a symbolic ref, and the time find_symref() calls\n> resolve_ref_unsafe() to find out where it leads to, if somebody else\n> updates HEAD, resolve_ref_unsafe() can give an unexpected result, as\n> all of these read-only operations are performed without any locking.\n> \n> And the unexpected discrepancy is reported by find_symref() as\n> fatal.  The server side dies, and somehow that fact is lost between\n> the upload-pack process and the client and somebody in the middle\n> (e.g. fastcgi interface or nginx webserver on the server side, or\n> the remote-curl helper on the client side) keeps the \"git fetch\"\n> process waiting.\n> \n> So there seem to be two issues.  \n> \n> - Because of the unlocked read, find_symref() can observe an\n>   inconsistent state.  Perhaps it should be updated not to die but\n>   to retry, expecting that transient inconsistency will go away.\n> \n> - A fatal error in upload-pack is not reported back to the client\n>   to cause it exit is an obvious one, and even if we find a way to\n>   make this fatal error in find_symref() not to trigger, fatal\n>   errors in other places in the code can trigger the same symptom.\n> \n\n"},{"id":"308221","messageId":"xmqqshphge7o.fsf@gitster.mtv.corp.google.com","threadId":"44750","inReplyTo":"xmqq8tr9huc0.fsf@gitster.mtv.corp.google.com","subject":"Re: Bug report: Git pull hang occasionally","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-21T21:32:59Z","receivedAt":"2016-12-21T21:33:10Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> And the unexpected discrepancy is reported by find_symref() as\n> fatal.  The server side dies, and somehow that fact is lost between\n> the upload-pack process and the client and somebody in the middle\n> (e.g. fastcgi interface or nginx webserver on the server side, or\n> the remote-curl helper on the client side) keeps the \"git fetch\"\n> process waiting.\n>\n> So there seem to be two issues.  \n>\n>  - Because of the unlocked read, find_symref() can observe an\n>    inconsistent state.  Perhaps it should be updated not to die but\n>    to retry, expecting that transient inconsistency will go away.\n>\n>  - A fatal error in upload-pack is not reported back to the client\n>    to cause it exit is an obvious one, and even if we find a way to\n>    make this fatal error in find_symref() not to trigger, fatal\n>    errors in other places in the code can trigger the same symptom.\n\nI wonder if the latter is solved by recent patch 296b847c0d\n(\"remote-curl: don't hang when a server dies before any output\",\n2016-11-18) on the client side.\n\n-- >8 --\nFrom: David Turner <dturner@twosigma.com>\nDate: Fri, 18 Nov 2016 15:30:49 -0500\nSubject: [PATCH] remote-curl: don't hang when a server dies before any output\n\nIn the event that a HTTP server closes the connection after giving a\n200 but before giving any packets, we don't want to hang forever\nwaiting for a response that will never come.  Instead, we should die\nimmediately.\n\nOne case where this happens is when attempting to fetch a dangling\nobject by its object name.  In this case, the server dies before\nsending any data.  Prior to this patch, fetch-pack would wait for\ndata from the server, and remote-curl would wait for fetch-pack,\ncausing a deadlock.\n\nDespite this patch, there is other possible malformed input that could\ncause the same deadlock (e.g. a half-finished pktline, or a pktline but\nno trailing flush).  There are a few possible solutions to this:\n\n1. Allowing remote-curl to tell fetch-pack about the EOF (so that\nfetch-pack could know that no more data is coming until it says\nsomething else).  This is tricky because an out-of-band signal would\nbe required, or the http response would have to be re-framed inside\nanother layer of pkt-line or something.\n\n2. Make remote-curl understand some of the protocol.  It turns out\nthat in addition to understanding pkt-line, it would need to watch for\nack/nak.  This is somewhat fragile, as information about the protocol\nwould end up in two places.  Also, pkt-lines which are already at the\nlength limit would need special handling.\n\nBoth of these solutions would require a fair amount of work, whereas\nthis hack is easy and solves at least some of the problem.\n\nStill to do: it would be good to give a better error message\nthan \"fatal: The remote end hung up unexpectedly\".\n\nSigned-off-by: David Turner <dturner@twosigma.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n remote-curl.c               |  8 ++++++++\n t/t5551-http-fetch-smart.sh | 30 ++++++++++++++++++++++++++++++\n 2 files changed, 38 insertions(+)\n\ndiff --git a/remote-curl.c b/remote-curl.c\nindex f14c41f4c0..ee4423659f 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -400,6 +400,7 @@ struct rpc_state {\n \tsize_t pos;\n \tint in;\n \tint out;\n+\tint any_written;\n \tstruct strbuf result;\n \tunsigned gzip_request : 1;\n \tunsigned initial_buffer : 1;\n@@ -456,6 +457,8 @@ static size_t rpc_in(char *ptr, size_t eltsize,\n {\n \tsize_t size = eltsize * nmemb;\n \tstruct rpc_state *rpc = buffer_;\n+\tif (size)\n+\t\trpc->any_written = 1;\n \twrite_or_die(rpc->in, ptr, size);\n \treturn size;\n }\n@@ -659,6 +662,8 @@ static int post_rpc(struct rpc_state *rpc)\n \tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, rpc_in);\n \tcurl_easy_setopt(slot->curl, CURLOPT_FILE, rpc);\n \n+\n+\trpc->any_written = 0;\n \terr = run_slot(slot, NULL);\n \tif (err == HTTP_REAUTH && !large_request) {\n \t\tcredential_fill(&http_auth);\n@@ -667,6 +672,9 @@ static int post_rpc(struct rpc_state *rpc)\n \tif (err != HTTP_OK)\n \t\terr = -1;\n \n+\tif (!rpc->any_written)\n+\t\terr = -1;\n+\n \tcurl_slist_free_all(headers);\n \tfree(gzip_body);\n \treturn err;\ndiff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\nindex 1ec5b2747a..43665ab4a8 100755\n--- a/t/t5551-http-fetch-smart.sh\n+++ b/t/t5551-http-fetch-smart.sh\n@@ -276,6 +276,36 @@ test_expect_success 'large fetch-pack requests can be split across POSTs' '\n \ttest_line_count = 2 posts\n '\n \n+test_expect_success 'test allowreachablesha1inwant' '\n+\ttest_when_finished \"rm -rf test_reachable.git\" &&\n+\tserver=\"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\" &&\n+\tmaster_sha=$(git -C \"$server\" rev-parse refs/heads/master) &&\n+\tgit -C \"$server\" config uploadpack.allowreachablesha1inwant 1 &&\n+\n+\tgit init --bare test_reachable.git &&\n+\tgit -C test_reachable.git remote add origin \"$HTTPD_URL/smart/repo.git\" &&\n+\tgit -C test_reachable.git fetch origin \"$master_sha\"\n+'\n+\n+test_expect_success 'test allowreachablesha1inwant with unreachable' '\n+\ttest_when_finished \"rm -rf test_reachable.git; git reset --hard $(git rev-parse HEAD)\" &&\n+\n+\t#create unreachable sha\n+\techo content >file2 &&\n+\tgit add file2 &&\n+\tgit commit -m two &&\n+\tgit push public HEAD:refs/heads/doomed &&\n+\tgit push public :refs/heads/doomed &&\n+\n+\tserver=\"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\" &&\n+\tmaster_sha=$(git -C \"$server\" rev-parse refs/heads/master) &&\n+\tgit -C \"$server\" config uploadpack.allowreachablesha1inwant 1 &&\n+\n+\tgit init --bare test_reachable.git &&\n+\tgit -C test_reachable.git remote add origin \"$HTTPD_URL/smart/repo.git\" &&\n+\ttest_must_fail git -C test_reachable.git fetch origin \"$(git rev-parse HEAD)\"\n+'\n+\n test_expect_success EXPENSIVE 'http can handle enormous ref negotiation' '\n \t(\n \t\tcd \"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\" &&\n-- \n2.11.0-442-g0c85c54a77\n\n"},{"id":"308224","messageId":"F2F25D8A-5DD5-4C62-8F8D-941AFDD874DC@netskope.com","threadId":"44750","inReplyTo":"xmqqshphge7o.fsf@gitster.mtv.corp.google.com","subject":"Re: Bug report: Git pull hang occasionally","fromName":"Kai Zhang","fromEmail":"kai@netskope.com","sentAt":"2016-12-21T22:31:44Z","receivedAt":"2016-12-21T22:31:54Z","isPatch":false,"sender":{"key":"kai@netskope.com","avatar":null},"body":"I will verify it. Thanks.\n> On Dec 21, 2016, at 1:32 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> \n> Junio C Hamano <gitster@pobox.com> writes:\n> \n>> And the unexpected discrepancy is reported by find_symref() as\n>> fatal.  The server side dies, and somehow that fact is lost between\n>> the upload-pack process and the client and somebody in the middle\n>> (e.g. fastcgi interface or nginx webserver on the server side, or\n>> the remote-curl helper on the client side) keeps the \"git fetch\"\n>> process waiting.\n>> \n>> So there seem to be two issues.  \n>> \n>> - Because of the unlocked read, find_symref() can observe an\n>>   inconsistent state.  Perhaps it should be updated not to die but\n>>   to retry, expecting that transient inconsistency will go away.\n>> \n>> - A fatal error in upload-pack is not reported back to the client\n>>   to cause it exit is an obvious one, and even if we find a way to\n>>   make this fatal error in find_symref() not to trigger, fatal\n>>   errors in other places in the code can trigger the same symptom.\n> \n> I wonder if the latter is solved by recent patch 296b847c0d\n> (\"remote-curl: don't hang when a server dies before any output\",\n> 2016-11-18) on the client side.\n> \n> -- >8 --\n> From: David Turner <dturner@twosigma.com>\n> Date: Fri, 18 Nov 2016 15:30:49 -0500\n> Subject: [PATCH] remote-curl: don't hang when a server dies before any output\n> \n> In the event that a HTTP server closes the connection after giving a\n> 200 but before giving any packets, we don't want to hang forever\n> waiting for a response that will never come.  Instead, we should die\n> immediately.\n> \n> One case where this happens is when attempting to fetch a dangling\n> object by its object name.  In this case, the server dies before\n> sending any data.  Prior to this patch, fetch-pack would wait for\n> data from the server, and remote-curl would wait for fetch-pack,\n> causing a deadlock.\n> \n> Despite this patch, there is other possible malformed input that could\n> cause the same deadlock (e.g. a half-finished pktline, or a pktline but\n> no trailing flush).  There are a few possible solutions to this:\n> \n> 1. Allowing remote-curl to tell fetch-pack about the EOF (so that\n> fetch-pack could know that no more data is coming until it says\n> something else).  This is tricky because an out-of-band signal would\n> be required, or the http response would have to be re-framed inside\n> another layer of pkt-line or something.\n> \n> 2. Make remote-curl understand some of the protocol.  It turns out\n> that in addition to understanding pkt-line, it would need to watch for\n> ack/nak.  This is somewhat fragile, as information about the protocol\n> would end up in two places.  Also, pkt-lines which are already at the\n> length limit would need special handling.\n> \n> Both of these solutions would require a fair amount of work, whereas\n> this hack is easy and solves at least some of the problem.\n> \n> Still to do: it would be good to give a better error message\n> than \"fatal: The remote end hung up unexpectedly\".\n> \n> Signed-off-by: David Turner <dturner@twosigma.com>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n> remote-curl.c               |  8 ++++++++\n> t/t5551-http-fetch-smart.sh | 30 ++++++++++++++++++++++++++++++\n> 2 files changed, 38 insertions(+)\n> \n> diff --git a/remote-curl.c b/remote-curl.c\n> index f14c41f4c0..ee4423659f 100644\n> --- a/remote-curl.c\n> +++ b/remote-curl.c\n> @@ -400,6 +400,7 @@ struct rpc_state {\n> \tsize_t pos;\n> \tint in;\n> \tint out;\n> +\tint any_written;\n> \tstruct strbuf result;\n> \tunsigned gzip_request : 1;\n> \tunsigned initial_buffer : 1;\n> @@ -456,6 +457,8 @@ static size_t rpc_in(char *ptr, size_t eltsize,\n> {\n> \tsize_t size = eltsize * nmemb;\n> \tstruct rpc_state *rpc = buffer_;\n> +\tif (size)\n> +\t\trpc->any_written = 1;\n> \twrite_or_die(rpc->in, ptr, size);\n> \treturn size;\n> }\n> @@ -659,6 +662,8 @@ static int post_rpc(struct rpc_state *rpc)\n> \tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, rpc_in);\n> \tcurl_easy_setopt(slot->curl, CURLOPT_FILE, rpc);\n> \n> +\n> +\trpc->any_written = 0;\n> \terr = run_slot(slot, NULL);\n> \tif (err == HTTP_REAUTH && !large_request) {\n> \t\tcredential_fill(&http_auth);\n> @@ -667,6 +672,9 @@ static int post_rpc(struct rpc_state *rpc)\n> \tif (err != HTTP_OK)\n> \t\terr = -1;\n> \n> +\tif (!rpc->any_written)\n> +\t\terr = -1;\n> +\n> \tcurl_slist_free_all(headers);\n> \tfree(gzip_body);\n> \treturn err;\n> diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\n> index 1ec5b2747a..43665ab4a8 100755\n> --- a/t/t5551-http-fetch-smart.sh\n> +++ b/t/t5551-http-fetch-smart.sh\n> @@ -276,6 +276,36 @@ test_expect_success 'large fetch-pack requests can be split across POSTs' '\n> \ttest_line_count = 2 posts\n> '\n> \n> +test_expect_success 'test allowreachablesha1inwant' '\n> +\ttest_when_finished \"rm -rf test_reachable.git\" &&\n> +\tserver=\"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\" &&\n> +\tmaster_sha=$(git -C \"$server\" rev-parse refs/heads/master) &&\n> +\tgit -C \"$server\" config uploadpack.allowreachablesha1inwant 1 &&\n> +\n> +\tgit init --bare test_reachable.git &&\n> +\tgit -C test_reachable.git remote add origin \"$HTTPD_URL/smart/repo.git\" &&\n> +\tgit -C test_reachable.git fetch origin \"$master_sha\"\n> +'\n> +\n> +test_expect_success 'test allowreachablesha1inwant with unreachable' '\n> +\ttest_when_finished \"rm -rf test_reachable.git; git reset --hard $(git rev-parse HEAD)\" &&\n> +\n> +\t#create unreachable sha\n> +\techo content >file2 &&\n> +\tgit add file2 &&\n> +\tgit commit -m two &&\n> +\tgit push public HEAD:refs/heads/doomed &&\n> +\tgit push public :refs/heads/doomed &&\n> +\n> +\tserver=\"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\" &&\n> +\tmaster_sha=$(git -C \"$server\" rev-parse refs/heads/master) &&\n> +\tgit -C \"$server\" config uploadpack.allowreachablesha1inwant 1 &&\n> +\n> +\tgit init --bare test_reachable.git &&\n> +\tgit -C test_reachable.git remote add origin \"$HTTPD_URL/smart/repo.git\" &&\n> +\ttest_must_fail git -C test_reachable.git fetch origin \"$(git rev-parse HEAD)\"\n> +'\n> +\n> test_expect_success EXPENSIVE 'http can handle enormous ref negotiation' '\n> \t(\n> \t\tcd \"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\" &&\n> -- \n> 2.11.0-442-g0c85c54a77\n> \n\n"},{"id":"309278","messageId":"E9196161-995C-4575-9560-D7E7B9A6B43D@netskope.com","threadId":"44750","inReplyTo":"xmqqshphge7o.fsf@gitster.mtv.corp.google.com","subject":"Re: Bug report: Git pull hang occasionally","fromName":"Kai Zhang","fromEmail":"kai@netskope.com","sentAt":"2017-01-12T18:24:03Z","receivedAt":"2017-01-12T18:33:38Z","isPatch":false,"sender":{"key":"kai@netskope.com","avatar":null},"body":"Hi Junio,\n\nAfter apply this patch, hanging did not happen again. Would this patch go to release in near future?\n\nThanks.\n\nRegards,\nKai \n> On Dec 21, 2016, at 1:32 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> \n> Junio C Hamano <gitster@pobox.com> writes:\n> \n>> And the unexpected discrepancy is reported by find_symref() as\n>> fatal.  The server side dies, and somehow that fact is lost between\n>> the upload-pack process and the client and somebody in the middle\n>> (e.g. fastcgi interface or nginx webserver on the server side, or\n>> the remote-curl helper on the client side) keeps the \"git fetch\"\n>> process waiting.\n>> \n>> So there seem to be two issues.  \n>> \n>> - Because of the unlocked read, find_symref() can observe an\n>>   inconsistent state.  Perhaps it should be updated not to die but\n>>   to retry, expecting that transient inconsistency will go away.\n>> \n>> - A fatal error in upload-pack is not reported back to the client\n>>   to cause it exit is an obvious one, and even if we find a way to\n>>   make this fatal error in find_symref() not to trigger, fatal\n>>   errors in other places in the code can trigger the same symptom.\n> \n> I wonder if the latter is solved by recent patch 296b847c0d\n> (\"remote-curl: don't hang when a server dies before any output\",\n> 2016-11-18) on the client side.\n> \n> -- >8 --\n> From: David Turner <dturner@twosigma.com>\n> Date: Fri, 18 Nov 2016 15:30:49 -0500\n> Subject: [PATCH] remote-curl: don't hang when a server dies before any output\n> \n> In the event that a HTTP server closes the connection after giving a\n> 200 but before giving any packets, we don't want to hang forever\n> waiting for a response that will never come.  Instead, we should die\n> immediately.\n> \n> One case where this happens is when attempting to fetch a dangling\n> object by its object name.  In this case, the server dies before\n> sending any data.  Prior to this patch, fetch-pack would wait for\n> data from the server, and remote-curl would wait for fetch-pack,\n> causing a deadlock.\n> \n> Despite this patch, there is other possible malformed input that could\n> cause the same deadlock (e.g. a half-finished pktline, or a pktline but\n> no trailing flush).  There are a few possible solutions to this:\n> \n> 1. Allowing remote-curl to tell fetch-pack about the EOF (so that\n> fetch-pack could know that no more data is coming until it says\n> something else).  This is tricky because an out-of-band signal would\n> be required, or the http response would have to be re-framed inside\n> another layer of pkt-line or something.\n> \n> 2. Make remote-curl understand some of the protocol.  It turns out\n> that in addition to understanding pkt-line, it would need to watch for\n> ack/nak.  This is somewhat fragile, as information about the protocol\n> would end up in two places.  Also, pkt-lines which are already at the\n> length limit would need special handling.\n> \n> Both of these solutions would require a fair amount of work, whereas\n> this hack is easy and solves at least some of the problem.\n> \n> Still to do: it would be good to give a better error message\n> than \"fatal: The remote end hung up unexpectedly\".\n> \n> Signed-off-by: David Turner <dturner@twosigma.com>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n> remote-curl.c               |  8 ++++++++\n> t/t5551-http-fetch-smart.sh | 30 ++++++++++++++++++++++++++++++\n> 2 files changed, 38 insertions(+)\n> \n> diff --git a/remote-curl.c b/remote-curl.c\n> index f14c41f4c0..ee4423659f 100644\n> --- a/remote-curl.c\n> +++ b/remote-curl.c\n> @@ -400,6 +400,7 @@ struct rpc_state {\n> \tsize_t pos;\n> \tint in;\n> \tint out;\n> +\tint any_written;\n> \tstruct strbuf result;\n> \tunsigned gzip_request : 1;\n> \tunsigned initial_buffer : 1;\n> @@ -456,6 +457,8 @@ static size_t rpc_in(char *ptr, size_t eltsize,\n> {\n> \tsize_t size = eltsize * nmemb;\n> \tstruct rpc_state *rpc = buffer_;\n> +\tif (size)\n> +\t\trpc->any_written = 1;\n> \twrite_or_die(rpc->in, ptr, size);\n> \treturn size;\n> }\n> @@ -659,6 +662,8 @@ static int post_rpc(struct rpc_state *rpc)\n> \tcurl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, rpc_in);\n> \tcurl_easy_setopt(slot->curl, CURLOPT_FILE, rpc);\n> \n> +\n> +\trpc->any_written = 0;\n> \terr = run_slot(slot, NULL);\n> \tif (err == HTTP_REAUTH && !large_request) {\n> \t\tcredential_fill(&http_auth);\n> @@ -667,6 +672,9 @@ static int post_rpc(struct rpc_state *rpc)\n> \tif (err != HTTP_OK)\n> \t\terr = -1;\n> \n> +\tif (!rpc->any_written)\n> +\t\terr = -1;\n> +\n> \tcurl_slist_free_all(headers);\n> \tfree(gzip_body);\n> \treturn err;\n> diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\n> index 1ec5b2747a..43665ab4a8 100755\n> --- a/t/t5551-http-fetch-smart.sh\n> +++ b/t/t5551-http-fetch-smart.sh\n> @@ -276,6 +276,36 @@ test_expect_success 'large fetch-pack requests can be split across POSTs' '\n> \ttest_line_count = 2 posts\n> '\n> \n> +test_expect_success 'test allowreachablesha1inwant' '\n> +\ttest_when_finished \"rm -rf test_reachable.git\" &&\n> +\tserver=\"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\" &&\n> +\tmaster_sha=$(git -C \"$server\" rev-parse refs/heads/master) &&\n> +\tgit -C \"$server\" config uploadpack.allowreachablesha1inwant 1 &&\n> +\n> +\tgit init --bare test_reachable.git &&\n> +\tgit -C test_reachable.git remote add origin \"$HTTPD_URL/smart/repo.git\" &&\n> +\tgit -C test_reachable.git fetch origin \"$master_sha\"\n> +'\n> +\n> +test_expect_success 'test allowreachablesha1inwant with unreachable' '\n> +\ttest_when_finished \"rm -rf test_reachable.git; git reset --hard $(git rev-parse HEAD)\" &&\n> +\n> +\t#create unreachable sha\n> +\techo content >file2 &&\n> +\tgit add file2 &&\n> +\tgit commit -m two &&\n> +\tgit push public HEAD:refs/heads/doomed &&\n> +\tgit push public :refs/heads/doomed &&\n> +\n> +\tserver=\"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\" &&\n> +\tmaster_sha=$(git -C \"$server\" rev-parse refs/heads/master) &&\n> +\tgit -C \"$server\" config uploadpack.allowreachablesha1inwant 1 &&\n> +\n> +\tgit init --bare test_reachable.git &&\n> +\tgit -C test_reachable.git remote add origin \"$HTTPD_URL/smart/repo.git\" &&\n> +\ttest_must_fail git -C test_reachable.git fetch origin \"$(git rev-parse HEAD)\"\n> +'\n> +\n> test_expect_success EXPENSIVE 'http can handle enormous ref negotiation' '\n> \t(\n> \t\tcd \"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\" &&\n> -- \n> 2.11.0-442-g0c85c54a77\n\n"},{"id":"309291","messageId":"xmqqy3yghtio.fsf@gitster.mtv.corp.google.com","threadId":"44750","inReplyTo":"E9196161-995C-4575-9560-D7E7B9A6B43D@netskope.com","subject":"Re: Bug report: Git pull hang occasionally","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-12T21:12:47Z","receivedAt":"2017-01-12T21:12:54Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kai Zhang <kai@netskope.com> writes:\n\n>> On Dec 21, 2016, at 1:32 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> \n>> Junio C Hamano <gitster@pobox.com> writes:\n>>  ...\n>> \n>> I wonder if the latter is solved by recent patch 296b847c0d\n>> (\"remote-curl: don't hang when a server dies before any output\",\n>> 2016-11-18) on the client side.\n>> ...\n\n> After apply this patch, hanging did not happen again. \n\nThanks for confirming.\n\n> Would this patch go to release in near future?\n\nI see 296b847c0d in the message you are responding to (by the way,\ndo not top-post to this list).  \n\nLet's check it together.\n\n\t$ git log master..296b847c0d\n\t$ git merge-base master 296b847c0d\n        296b847c0d6de63353e236cfbf94163d24155529\n\nYup, it already is in master.  \n\nUsing a third-party script \"when-merged\" [*1*], we can easily find\nthat it was merged a few days ago to 'master', after cooking in\n'next' for a handful of weeks:\n\n\t$ git when-merged 296b847c0d next\n\trefs/heads/next 3ea70d01afc6305b88d33b8585f1fc41c486a182\n\t$ git when-merged 296b847c0d master\n\trefs/heads/master d984592043aec3c9f5b1955560a133896ca115b5\n\t$ git show -s --format='%cI' 3ea70d01af d984592043 \n        2016-12-05T11:38:03-08:00\n        2017-01-10T15:24:25-08:00\n\nUnless people find regressions caused by this change (in which case\nwe may have to revert it), this will be included in the release at\nthe end of this cycle.  http://tinyurl.com/gitCal tells us that the\ncurrent cycle is expected to complete early February.\n\n\n[Footnote]\n\n*1* git://github.com/mhagger/git-when-merged\n"},{"id":"309292","messageId":"62D2BA2E-9A15-4D6E-B232-1FDFEC1B9033@netskope.com","threadId":"44750","inReplyTo":"xmqqy3yghtio.fsf@gitster.mtv.corp.google.com","subject":"Re: Bug report: Git pull hang occasionally","fromName":"Kai Zhang","fromEmail":"kai@netskope.com","sentAt":"2017-01-12T21:17:16Z","receivedAt":"2017-01-12T21:17:38Z","isPatch":false,"sender":{"key":"kai@netskope.com","avatar":null},"body":"\n> On Jan 12, 2017, at 1:12 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> \n> Kai Zhang <kai@netskope.com> writes:\n> \n>>> On Dec 21, 2016, at 1:32 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>>> \n>>> Junio C Hamano <gitster@pobox.com> writes:\n>>> ...\n>>> \n>>> I wonder if the latter is solved by recent patch 296b847c0d\n>>> (\"remote-curl: don't hang when a server dies before any output\",\n>>> 2016-11-18) on the client side.\n>>> ...\n> \n>> After apply this patch, hanging did not happen again. \n> \n> Thanks for confirming.\n> \n>> Would this patch go to release in near future?\n> \n> I see 296b847c0d in the message you are responding to (by the way,\n> do not top-post to this list).  \n> \n> Let's check it together.\n> \n> \t$ git log master..296b847c0d\n> \t$ git merge-base master 296b847c0d\n>        296b847c0d6de63353e236cfbf94163d24155529\n> \n> Yup, it already is in master.  \n> \n> Using a third-party script \"when-merged\" [*1*], we can easily find\n> that it was merged a few days ago to 'master', after cooking in\n> 'next' for a handful of weeks:\n> \n> \t$ git when-merged 296b847c0d next\n> \trefs/heads/next 3ea70d01afc6305b88d33b8585f1fc41c486a182\n> \t$ git when-merged 296b847c0d master\n> \trefs/heads/master d984592043aec3c9f5b1955560a133896ca115b5\n> \t$ git show -s --format='%cI' 3ea70d01af d984592043 \n>        2016-12-05T11:38:03-08:00\n>        2017-01-10T15:24:25-08:00\n> \n> Unless people find regressions caused by this change (in which case\n> we may have to revert it), this will be included in the release at\n> the end of this cycle.  http://tinyurl.com/gitCal tells us that the\n> current cycle is expected to complete early February.\n> \n> \n> [Footnote]\n> \n> *1* git://github.com/mhagger/git-when-merged\n\nThank you for your help!\n"}]}