Re: [PATCH v2 0/7] remote-curl: fix deadlocks when remote server disconnects

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

 



On Mon, May 18, 2020 at 03:52:42PM -0700, Junio C Hamano wrote:

> Jeff King <peff@xxxxxxxx> writes:
> 
> > So I think our options are probably:
> >
> >   1. detect flush packets in remote-curl, and either:
> >
> >      a. don't print an error, just hang up. That prevents a hang in the
> > 	caller and produces no extra message on a real error. It may be
> > 	less informative than it could be if the connection hangs up
> > 	(though we may print a curl error message, and the caller will
> > 	at least say "the helper hung up")
> >
> >      b. like (a), but always print an error; this is your original
> > 	patch, but I _suspect_ (but didn't test) that it would produce
> > 	extra useless messages for errors the server reports
> >
> >      c. between the two: inspect the final packet data for evidence of
> >         ERR/sideband 3 and suppress any message if found
> >
> >   2. helper signals end-of-response to caller (then it never produces a
> >      message itself; only the caller does, and it would abort on an ERR
> >      packet before then)
> >
> >      a. using a special pktline (your "0002" patch)
> >
> >      b. some other out-of-band mechanism (e.g., could be another fd)
> >
> > I think this is pushing me towards 2a, your "0002" patch. It sidesteps
> > the error-message questions entirely (and I think 2b is too convoluted
> > to be worth pursuing, especially on Windows where setting up extra pipes
> > is tricky). But I'd also be OK with 1a or 1c.
> 
> Thanks for a detailed analysis.  I guess we'd take 0002, then?

Yeah, that's how I lean. I think I did have a few comments on the patch
that I'd like Denton to consider, so hopefully we'll see a re-roll.

-Peff



[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