From: Jeff King <peff@peff.net>
To: Denton Liu <liu.denton@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>,
Brandon Williams <bmwill@google.com>,
Jonathan Tan <jonathantanmy@google.com>
Subject: Re: [RFC PATCH] http-backend: write error packet if backend command fails
Date: Sat, 28 Mar 2020 11:13:00 -0400 [thread overview]
Message-ID: <20200328151300.GA1215566@coredump.intra.peff.net> (raw)
In-Reply-To: <20200328145741.GA1209174@coredump.intra.peff.net>
On Sat, Mar 28, 2020 at 10:57:42AM -0400, Jeff King wrote:
> But since it works for v1 (which also dies!), and since you were able to
> reproduce the problem locally, I suspect it may be an issue in
> upload-pack itself.
Actually, I think the problem is on the client side.
If I run your test without the http-backend change, then nothing is left
running on the server side, but on the client we have two processes:
git-clone and the git-remote-https helper. And they're deadlocked trying
to read from each other.
I think the issue is that git-remote-https in v2 mode calls into
stateless_connect(), and just pumps http request/response pairs from
git-clone to the server. But if a request generates an empty response,
then clone has no idea that anything was received at all. It continues
waiting, but remote-https has looped to expect another request from it.
Your patch fixes _this case_ because it causes the server to send a
non-empty response. But the client can't rely on that. Besides that not
being a documented server behavior, in the worst case the connection
could be severed mid-stream. So I think remote-https needs to complain
about an empty response. This isn't a problem in v1 because it would
actually try to look at that empty response; in v2 it's just proxying
bytes around.
-Peff
next prev parent reply other threads:[~2020-03-28 15:13 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-28 2:50 [RFC PATCH] http-backend: write error packet if backend command fails Denton Liu
2020-03-28 14:57 ` Jeff King
2020-03-28 15:13 ` Jeff King [this message]
2020-03-28 15:49 ` Jeff King
2020-03-29 5:34 ` Denton Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200328151300.GA1215566@coredump.intra.peff.net \
--to=peff@peff.net \
--cc=bmwill@google.com \
--cc=git@vger.kernel.org \
--cc=jonathantanmy@google.com \
--cc=liu.denton@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).