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

From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Denton Liu <liu.denton@gmail.com>,
	Git Mailing List <git@vger.kernel.org>,
	Eric Sunshine <sunshine@sunshineco.com>
Subject: Re: [PATCH v2 0/7] remote-curl: fix deadlocks when remote server disconnects
Date: Mon, 18 May 2020 15:52:42 -0700	[thread overview]
Message-ID: <xmqqh7wc26v9.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <20200518205854.GB63978@coredump.intra.peff.net> (Jeff King's message of "Mon, 18 May 2020 16:58:54 -0400")

Jeff King <peff@peff.net> 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?