Re: [PATCHv3 2/9] send-pack: Attempt to retrieve remote status even if pack-objects fails

On Sun, May 15, 2011 at 11:37:13PM +0200, Johan Herland wrote:

I first wrote this patch on a base where e07fd15 (Peff's "send-pack:
unbreak push over stateless rpc") was not present, and then resolved
a conflict when rebasing this patch onto current master. I hope Peff
or Johannes (Sixt) can verify that my patch does not reintroduce the
deadlock they fixed.

I don't think it reintroduces the deadlock we fixed, but I am worried
that it produces a new, similar one. That is, imagine pack-objects fails
horribly, maybe or maybe not producing any output. We close its pipe
outgoing pipe at the end of run_command, and per Johannes' 09c9957, we
are sure that the sideband demuxer does not hold a pipe end open,
either.

So the remote side sees us close our end of the pipe, knows there is no
more pack data, and then closes their end. So our receive_status should
either get some error message, or EOF, either of which is fine. So no
deadlock there. Essentially, we have done a half-duplex shutdown of the
connection to the remote, and that is enough for everybody to keep
going.

But what if we are not using pipes, but have an actual TCP socket? In
that case, I'm not sure what happens. We don't seem to do a half-duplex
shutdown() anywhere. So I'm concerned that we are still open for sending
from the remote's perspective, and we may deadlock.

However, that would not necessarily be something introduced by your
patch; you would deadlock in receive_status, but prior to that it would
deadlock in the sideband demuxer.

AFAICT, the only way to have an actual TCP connection instead of pipes
is for the push to go over git://, which is enabled almost nowhere. But
we should perhaps check for deadlock on failed pack-objects in that
case, both with and without your patch.

-Peff