Re: [PATCH 1/1] upload-pack: fix race condition in error messages

[Jeff King <peff@peff.net>]

On Thu, Aug 29, 2019 at 10:27:16AM -0400, Derrick Stolee wrote:

> > I don't think we should need such a call. For one thing, if it were
> > necessary, that would mean we're not writing out the packet at all. But
> > your whole problem is that we're writing the message twice, one of which
> > comes from the packet.
> 
> The problem the flush() was trying to solve was the new "Broken pipe" error,
> which I had assumed was due to a communication race. (Looking at the message
> more closely now, I see that Szeder was able to repro this broken pipe both
> with and without my change. I am still unable to repro the broken pipe.)

I think the broken pipe is coming the _other_ way. We do send the packet
from the server to the client, but since the client is still writing
when the server has hung up, we get a write error instead of seeing the
error packet.

So any fixes there have to happen on the client side. I am still
confused about why the client is writing in this case, per the argument
in 014ade7484 (upload-pack: send ERR packet for non-tip objects,
2019-04-13). It would be nice to use GIT_TRACE_PACKET to see what it's
trying to write, but I still haven't been able to reproduce the issue.

-Peff