Re: [PATCH v19 03/10] receive-pack: add new proc-receive hook

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



SZEDER Gábor <szeder.dev@xxxxxxxxx> 于2021年1月20日周三 下午8:29写道:
>
> On Mon, Jan 18, 2021 at 04:24:11PM +0800, Jiang Xin wrote:
> > SZEDER Gábor <szeder.dev@xxxxxxxxx> 于2021年1月18日周一 上午6:21写道:
> > >
> > >
> > > This patch added a whole lot of test cases, and two of them '86 -
> > > proc-receive: not support push options (builtin protocol)' and '95 -
> > > proc-receive: not support push options (builtin protocol/porcelain)'
> > > are prone to rare failures.
> > >
> > > On Thu, Aug 27, 2020 at 11:45:44AM -0400, Jiang Xin wrote:
> > > > diff --git a/t/t5411/test-0026-push-options.sh b/t/t5411/test-0026-push-options.sh
> > > > new file mode 100644
> > > > index 0000000000..d0c4da8b23
> > > > --- /dev/null
> > > > +++ b/t/t5411/test-0026-push-options.sh
> > >
> > > > +# Refs of upstream : master(A)
> > > > +# Refs of workbench: master(A)  tags/v123
> > > > +# git push -o ...  :                       refs/for/master/topic
> > > > +test_expect_success "proc-receive: not support push options ($PROTOCOL)" '
> > > > +     test_must_fail git -C workbench push \
> > > > +             -o issue=123 \
> > > > +             -o reviewer=user1 \
> > > > +             origin \
> > > > +             HEAD:refs/for/master/topic \
> > > > +             >out 2>&1 &&
> > >
> > > Three relevant things are happening here:
> > >
> > >   - 'git push' is executed with its standard output and error
> > >     redirected to the file 'out'.
> > >
> > >   - 'git push' executes 'git receive-pack' internally, which inherits
> > >     the open file descriptors, so its output and error goes into that
> > >     same 'out' file.
> > >
> > >   - 'git push' is expected to fail when it finds out that the other
> > >     side doesn't support push options, but it does so with a simple
> > >     die() right away, without waiting for its child 'git receive-pack'
> > >     process to finish.
> > >
> > > > +     make_user_friendly_and_stable_output <out >actual &&
> > > > +     test_i18ngrep "fatal: the receiving end does not support push options" \
> > > > +             actual &&
> > > > +     git -C "$upstream" show-ref >out &&
> > >
> > > Here the shell opens and truncates the file 'out' to write 'git
> > > show-ref's output, i.e. it is still the same 'out' file that was used
> > > earlier.
> > >
> > > Consequently, it is possible that 'git receive-pack' is still running,
> > > its open file descriptors to 'out' are still valid, and its "fatal:
> > > the remote end hung up unexpectedly" error message about the suddenly
> > > disappeared 'git push' can partially overwrite the output from 'git
> > > show-ref'.
>
>
> > > I think these are the only two tests that can cause this racy
> > > behavior: by instrumenting finish_command() I found that in all other
> > > tests where 'git push' is expected to fail it errors out gracefully
> > > and waits for its 'git receive-pack' child process.
> >
> > Atomic push may have the same problem.
>
> I don't think so, because send_pack() doesn't die() when a ref is
> rejected in an atomic push, but returns, and lets its caller terminate
> in an usual way, including waiting for 'git receive-pack'.

I find many places where the client side will die() before closing the
service side gracefully:

 + In `transport_push()`, if fail to push a submodule, will die().
 + In `git_transport_push()`, will die() for an unimplemented v2 protocol.
 + In `send_pack()`, will die() if
    the server side has an incompatible hash algorithm, or
    the receiving end does not support --signed push, or
    the receiving end does not support --atomic push, or
    the receiving end does not support push options




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux