Problem accessing git.kernel.org with git v2.33 plus gitproxy

Problem accessing git.kernel.org with git v2.33 plus gitproxy

[Kirill A. Shutemov]

Hi folks,

I've stepped on a problem after upgrading git to v2.33.0. git fetch-pack
fails with an error:

        fetch-pack: unexpected disconnect while reading sideband packet

It only happens when I access git.kernel.org over git:// (github over
git:// works fine) and if there's a gitproxy configured.

For test I used a dummy gitproxy:

        #!/bin/sh -efu
        socat - "TCP:$1:$2"

It is enough to trigger the issue.

I'm not sure if it's kernel.org problem or git problem.

Has anybody else stepped on the issue? Any clues?

-- 
 Kirill A. Shutemov

Re: Problem accessing git.kernel.org with git v2.33 plus gitproxy

[Konstantin Ryabitsev]

On Mon, Aug 30, 2021 at 07:11:49PM +0300, Kirill A. Shutemov wrote:
> I've stepped on a problem after upgrading git to v2.33.0. git fetch-pack
> fails with an error:
> 
>         fetch-pack: unexpected disconnect while reading sideband packet
> 
> It only happens when I access git.kernel.org over git:// (github over
> git:// works fine) and if there's a gitproxy configured.
> 
> For test I used a dummy gitproxy:
> 
>         #!/bin/sh -efu
>         socat - "TCP:$1:$2"
> 
> It is enough to trigger the issue.
> 
> I'm not sure if it's kernel.org problem or git problem.

For the record, the git.kernel.org server-side version is 2.31.1 and it is
invoked via the usual xinetd straight to git-daemon with the following flags:

server_args = --interpolated-path=/var/lib/git/%H%D --init-timeout=10 --timeout=600 --export-all --syslog --inetd --verbose

-K

Re: Problem accessing git.kernel.org with git v2.33 plus gitproxy

[Pratyush Yadav]

On 30/08/21 07:11PM, Kirill A. Shutemov wrote:
> Hi folks,
> 
> I've stepped on a problem after upgrading git to v2.33.0. git fetch-pack
> fails with an error:
> 
>         fetch-pack: unexpected disconnect while reading sideband packet
> 
> It only happens when I access git.kernel.org over git:// (github over
> git:// works fine) and if there's a gitproxy configured.
> 
> For test I used a dummy gitproxy:
> 
>         #!/bin/sh -efu
>         socat - "TCP:$1:$2"
> 
> It is enough to trigger the issue.
> 
> I'm not sure if it's kernel.org problem or git problem.
> 
> Has anybody else stepped on the issue? Any clues?

I can reproduce it on my computer at work, which needs to sit behind a 
proxy. Not too sure what is causing it though.

-- 
Regards,
Pratyush Yadav

Re: Problem accessing git.kernel.org with git v2.33 plus gitproxy

[Kirill A. Shutemov]

On Mon, Aug 30, 2021 at 07:11:50PM +0300, Kirill A. Shutemov wrote:
> Hi folks,
> 
> I've stepped on a problem after upgrading git to v2.33.0. git fetch-pack
> fails with an error:
> 
>         fetch-pack: unexpected disconnect while reading sideband packet
> 
> It only happens when I access git.kernel.org over git:// (github over
> git:// works fine) and if there's a gitproxy configured.
> 
> For test I used a dummy gitproxy:
> 
>         #!/bin/sh -efu
>         socat - "TCP:$1:$2"
> 
> It is enough to trigger the issue.
> 
> I'm not sure if it's kernel.org problem or git problem.
> 
> Has anybody else stepped on the issue? Any clues?

I've bisected the issue to commit

	ae1a7eefffe6 ("fetch-pack: signal v2 server that we are done making requests")

-- 
 Kirill A. Shutemov

Re: Problem accessing git.kernel.org with git v2.33 plus gitproxy

[Andy Shevchenko]

On Mon, Aug 30, 2021 at 09:04:33PM +0300, Kirill A. Shutemov wrote:
> On Mon, Aug 30, 2021 at 07:11:50PM +0300, Kirill A. Shutemov wrote:
> > Hi folks,
> > 
> > I've stepped on a problem after upgrading git to v2.33.0. git fetch-pack
> > fails with an error:
> > 
> >         fetch-pack: unexpected disconnect while reading sideband packet
> > 
> > It only happens when I access git.kernel.org over git:// (github over
> > git:// works fine) and if there's a gitproxy configured.
> > 
> > For test I used a dummy gitproxy:
> > 
> >         #!/bin/sh -efu
> >         socat - "TCP:$1:$2"
> > 
> > It is enough to trigger the issue.
> > 
> > I'm not sure if it's kernel.org problem or git problem.
> > 
> > Has anybody else stepped on the issue? Any clues?
> 
> I've bisected the issue to commit
> 
> 	ae1a7eefffe6 ("fetch-pack: signal v2 server that we are done making requests")

Thanks, Kirill!

Are you going to report it to the Git community?

-- 
With Best Regards,
Andy Shevchenko

Re: Problem accessing git.kernel.org with git v2.33 plus gitproxy

[Andy Shevchenko]

On Mon, Aug 30, 2021 at 09:29:54PM +0300, Andy Shevchenko wrote:
> On Mon, Aug 30, 2021 at 09:04:33PM +0300, Kirill A. Shutemov wrote:
> > On Mon, Aug 30, 2021 at 07:11:50PM +0300, Kirill A. Shutemov wrote:
> > > Hi folks,
> > > 
> > > I've stepped on a problem after upgrading git to v2.33.0. git fetch-pack
> > > fails with an error:
> > > 
> > >         fetch-pack: unexpected disconnect while reading sideband packet
> > > 
> > > It only happens when I access git.kernel.org over git:// (github over
> > > git:// works fine) and if there's a gitproxy configured.
> > > 
> > > For test I used a dummy gitproxy:
> > > 
> > >         #!/bin/sh -efu
> > >         socat - "TCP:$1:$2"
> > > 
> > > It is enough to trigger the issue.
> > > 
> > > I'm not sure if it's kernel.org problem or git problem.
> > > 
> > > Has anybody else stepped on the issue? Any clues?
> > 
> > I've bisected the issue to commit
> > 
> > 	ae1a7eefffe6 ("fetch-pack: signal v2 server that we are done making requests")
> 
> Thanks, Kirill!
> 
> Are you going to report it to the Git community?

Just realized that this discussion is in the Git community :-)

-- 
With Best Regards,
Andy Shevchenko

Re: Problem accessing git.kernel.org with git v2.33 plus gitproxy

[Jeff King]

On Mon, Aug 30, 2021 at 07:11:49PM +0300, Kirill A. Shutemov wrote:

> I've stepped on a problem after upgrading git to v2.33.0. git fetch-pack
> fails with an error:
> 
>         fetch-pack: unexpected disconnect while reading sideband packet
> 
> It only happens when I access git.kernel.org over git:// (github over
> git:// works fine) and if there's a gitproxy configured.
> 
> For test I used a dummy gitproxy:
> 
>         #!/bin/sh -efu
>         socat - "TCP:$1:$2"
> 
> It is enough to trigger the issue.
> 
> I'm not sure if it's kernel.org problem or git problem.
> 
> Has anybody else stepped on the issue? Any clues?

I can't reproduce the problem here, using core.gitproxy with a script
identical to what you showed above. I tried both cloning, and fetching
via both git-fetch and git-fetch-pack.

Can you show us a more complete example? What does the command that
fails look like? What's the repo state before you run it? At what part
of the conversation does it fail (before a pack is sent, during, or
after)?

If you can reproduce it at will and it fails on 2.33 but not earlier,
then bisecting might be helpful.

-Peff

Re: Problem accessing git.kernel.org with git v2.33 plus gitproxy

[Kirill A. Shutemov]

On Mon, Aug 30, 2021 at 02:16:04PM -0400, Jeff King wrote:
> On Mon, Aug 30, 2021 at 07:11:49PM +0300, Kirill A. Shutemov wrote:
> 
> > I've stepped on a problem after upgrading git to v2.33.0. git fetch-pack
> > fails with an error:
> > 
> >         fetch-pack: unexpected disconnect while reading sideband packet
> > 
> > It only happens when I access git.kernel.org over git:// (github over
> > git:// works fine) and if there's a gitproxy configured.
> > 
> > For test I used a dummy gitproxy:
> > 
> >         #!/bin/sh -efu
> >         socat - "TCP:$1:$2"
> > 
> > It is enough to trigger the issue.
> > 
> > I'm not sure if it's kernel.org problem or git problem.
> > 
> > Has anybody else stepped on the issue? Any clues?
> 
> I can't reproduce the problem here, using core.gitproxy with a script
> identical to what you showed above. I tried both cloning, and fetching
> via both git-fetch and git-fetch-pack.

Could you try with a kernel repo?

git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

I found that not all repos on kernel.org trigger the issue.

> Can you show us a more complete example? What does the command that
> fails look like? What's the repo state before you run it? At what part
> of the conversation does it fail (before a pack is sent, during, or
> after)?

The last I see sent to the server is "packfile". It is consistent with the
bisected commit. Removing close() in do_fetch_pack_v2() helps.

> If you can reproduce it at will and it fails on 2.33 but not earlier,
> then bisecting might be helpful.

I did. See my other mail.

-- 
 Kirill A. Shutemov

Re: Problem accessing git.kernel.org with git v2.33 plus gitproxy

[Jeff King]

On Mon, Aug 30, 2021 at 09:28:45PM +0300, Kirill A. Shutemov wrote:

> > I can't reproduce the problem here, using core.gitproxy with a script
> > identical to what you showed above. I tried both cloning, and fetching
> > via both git-fetch and git-fetch-pack.
> 
> Could you try with a kernel repo?
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> 
> I found that not all repos on kernel.org trigger the issue.

Thanks, I was able to reproduce there (but not with git.git). That makes
me wonder if it's a race condition of some sort. My reproduction was
just:

  git init
  git config core.gitproxy /path/to/proxy/script
  git fetch-pack \
    git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git \
    refs/heads/master

In the meantime, a workaround is:

  git -c protocol.version=0 fetch-pack ...

> > If you can reproduce it at will and it fails on 2.33 but not earlier,
> > then bisecting might be helpful.
> 
> I did. See my other mail.

Yeah, looks like you found ae1a7eefff (fetch-pack: signal v2 server that
we are done making requests, 2021-05-19). I suspected that might be the
case.

I strace'd the underlying socat, and it does this (numbers on the left
are my annotations):

     select(6, [0 5], [1], [], NULL)         = 2 (in [0], out [1])
     recvfrom(3, 0x7ffee69f1f50, 519, MSG_DONTWAIT, NULL, NULL) = -1 EAGAIN (Resource temporarily unavailable)
     recvfrom(3, 0x7ffee69f19d0, 519, MSG_DONTWAIT, NULL, NULL) = -1 EAGAIN (Resource temporarily unavailable)
[0]  read(0, "", 8192)                       = 0
     recvfrom(3, 0x7ffee69f19d0, 519, MSG_DONTWAIT, NULL, NULL) = -1 EAGAIN (Resource temporarily unavailable)
[1]  shutdown(5, SHUT_WR)                    = 0
     recvfrom(3, 0x7ffee69f1f50, 519, MSG_DONTWAIT, NULL, NULL) = -1 EAGAIN (Resource temporarily unavailable)
[2]  select(6, [5], [], [], {tv_sec=0, tv_usec=500000}) = 0 (Timeout)
     recvfrom(3, 0x7ffee69f1f50, 519, MSG_DONTWAIT, NULL, NULL) = -1 EAGAIN (Resource temporarily unavailable)
[3]  shutdown(5, SHUT_RDWR)                  = 0
     recvfrom(3, 0x7ffee69f2240, 519, MSG_DONTWAIT, NULL, NULL) = -1 EAGAIN (Resource temporarily unavailable)
     exit_group(0)                           = ?

Here descriptors 0/1 are the pipes from/to Git, and 5 is the TCP socket
connected to the server. The recvfrom() is just noise, I think; socat
opens a dgram socketpair(), but doesn't seem to do anything with it.

So in [0] we see that Git has hung up half of the pipe, due to the new
code in ae1a7eefff. socat then correctly relays the half-duplex shutdown
to the server in [1]. At this point it should wait for the server to
send the data, and relay it to stdout. And indeed, it does call select()
in [2]. But then when it hits the half-second timeout, it shuts down
completely!

I'm not that familiar with socat, but I've seen the same thing with
older versions of netcat: it wants to quit after seeing EOF on stdin.
This is useful to prevent deadlock if the server doesn't respond to a
half-duplex shutdown. But it's quite the wrong thing to do for a more
intelligent protocol.

That explains why you see the problem sometimes but not others. It
depends how long the server takes before it produces any output, which
in turn may depend on things like repo size. You said you didn't see it
when fetching from GitHub, but I suspect it is simply that GitHub's
server responds a little bit more quickly.

In netcat, the fix is to use "-q" (though at least some versions of
netcat will wait forever by default these days, so it's not a problem).
In socat, it looks like "-t" does the same thing. And indeed, switching
the proxy to:

  socat -t 10 - "TCP:$1:$2"

makes the problem go away for me. The 10-second timeout might seem
arbitrary, but it should be reliable. Git's server-side has a keep-alive
mechanism that sends a packet every 5 seconds, even if no output has
been produced yet. So even if the request takes a long time to generate
any output, it should be plenty to tell socat that the connection is
still live.

I am sympathetic that this used to work, and now doesn't. But this proxy
case is affected by the problem that ae1a7eefff was solving. The root of
the issue is just that "socat" in its default form is not doing the
right thing. So I'd prefer not to try to make any change to Git's
behavior here. But one option would be to limit it only to ssh, and not
git:// proxies (we already don't do that half-duplex shutdown for raw
TCP git://, for reasons discussed in that commit message).

-Peff

Re: Problem accessing git.kernel.org with git v2.33 plus gitproxy

[Kirill A. Shutemov]

On Mon, Aug 30, 2021 at 04:37:50PM -0400, Jeff King wrote:
> I am sympathetic that this used to work, and now doesn't. But this proxy
> case is affected by the problem that ae1a7eefff was solving. The root of
> the issue is just that "socat" in its default form is not doing the
> right thing. So I'd prefer not to try to make any change to Git's
> behavior here.

As a kernel developer I learned hard way that breaking user experience by
kernel changes considered a kernel regression, even if userspace "does it
wrong"™. I'm not sure what standard of care for Git users is.

I'm fine adjusting the proxy script and make my colleagues aware about the
issue, but the approach doesn't scale.

> But one option would be to limit it only to ssh, and not
> git:// proxies (we already don't do that half-duplex shutdown for raw
> TCP git://, for reasons discussed in that commit message).

I wounder if it's possible to detect the situation, warn the user that
gitproxy has to be fixed and retry fetching pack without closing fd[1].

-- 
 Kirill A. Shutemov

Re: Problem accessing git.kernel.org with git v2.33 plus gitproxy

[Jeff King]

On Tue, Aug 31, 2021 at 01:42:15AM +0300, Kirill A. Shutemov wrote:

> On Mon, Aug 30, 2021 at 04:37:50PM -0400, Jeff King wrote:
> > I am sympathetic that this used to work, and now doesn't. But this proxy
> > case is affected by the problem that ae1a7eefff was solving. The root of
> > the issue is just that "socat" in its default form is not doing the
> > right thing. So I'd prefer not to try to make any change to Git's
> > behavior here.
> 
> As a kernel developer I learned hard way that breaking user experience by
> kernel changes considered a kernel regression, even if userspace "does it
> wrong"™. I'm not sure what standard of care for Git users is.
> 
> I'm fine adjusting the proxy script and make my colleagues aware about the
> issue, but the approach doesn't scale.

I think we're a little less extreme there than the kernel. Like I said,
my preference is to leave Git as-is, but if somebody feels strongly, I
don't think it would be that hard to leave core.gitproxy untouched here.

I agree it doesn't scale, but my suspicion is that we're talking about
an extremely small population here. IMHO we should be considering
deprecating git:// entirely (from Git itself, and kernel.org should
consider turning it off). In the v2 protocol, there's no advantage to
using it over HTTP.

> > But one option would be to limit it only to ssh, and not
> > git:// proxies (we already don't do that half-duplex shutdown for raw
> > TCP git://, for reasons discussed in that commit message).
> 
> I wounder if it's possible to detect the situation, warn the user that
> gitproxy has to be fixed and retry fetching pack without closing fd[1].

I don't think it can be easily distinguished from an actual network
hangup (or proxy command failure, etc). I would much rather stop doing
the close() entirely than add any kind of heuristic retry.

-Peff

Re: Problem accessing git.kernel.org with git v2.33 plus gitproxy

[Andy Shevchenko]

On Tue, Aug 31, 2021 at 02:03:30AM -0400, Jeff King wrote:
> On Tue, Aug 31, 2021 at 01:42:15AM +0300, Kirill A. Shutemov wrote:
> > On Mon, Aug 30, 2021 at 04:37:50PM -0400, Jeff King wrote:
> > > I am sympathetic that this used to work, and now doesn't. But this proxy
> > > case is affected by the problem that ae1a7eefff was solving. The root of
> > > the issue is just that "socat" in its default form is not doing the
> > > right thing. So I'd prefer not to try to make any change to Git's
> > > behavior here.
> > 
> > As a kernel developer I learned hard way that breaking user experience by
> > kernel changes considered a kernel regression, even if userspace "does it
> > wrong"™. I'm not sure what standard of care for Git users is.
> > 
> > I'm fine adjusting the proxy script and make my colleagues aware about the
> > issue, but the approach doesn't scale.
> 
> I think we're a little less extreme there than the kernel. Like I said,
> my preference is to leave Git as-is, but if somebody feels strongly, I
> don't think it would be that hard to leave core.gitproxy untouched here.

So far the massive (annoying) Git breakage happened second time to me (*).
I would agree with you if it's rare to happen, to me it's like once per
~2 years. So, can you define what the tolerable period of time is when
I have to expect a Git breakage next time?

(*) First time it was a few years ago when out of a sudden Git started to pull
gigabytes of repositories without any need. It was annoying, but tolerable to
some extent. Current situation is not better form my user perspective.

> I agree it doesn't scale, but my suspicion is that we're talking about
> an extremely small population here. IMHO we should be considering
> deprecating git:// entirely (from Git itself, and kernel.org should
> consider turning it off). In the v2 protocol, there's no advantage to
> using it over HTTP.

So far don't you need to support current use cases?

(Here of course a philosophical question: is a driver moves a car, or the car moves the driver)

> > > But one option would be to limit it only to ssh, and not
> > > git:// proxies (we already don't do that half-duplex shutdown for raw
> > > TCP git://, for reasons discussed in that commit message).
> > 
> > I wounder if it's possible to detect the situation, warn the user that
> > gitproxy has to be fixed and retry fetching pack without closing fd[1].
> 
> I don't think it can be easily distinguished from an actual network
> hangup (or proxy command failure, etc). I would much rather stop doing
> the close() entirely than add any kind of heuristic retry.

-- 
With Best Regards,
Andy Shevchenko