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

[Jeff King <peff@peff.net>]

On Mon, May 16, 2011 at 02:13:54AM -0400, Jeff King wrote:

> and this patch fixes it:
> 
> diff --git a/builtin/send-pack.c b/builtin/send-pack.c
> index e2f4e21..b9da044 100644
> --- a/builtin/send-pack.c
> +++ b/builtin/send-pack.c
> @@ -345,6 +345,13 @@ int send_pack(struct send_pack_args *args,
>  				ref->status = REF_STATUS_NONE;
>  			if (args->stateless_rpc)
>  				close(out);
> +			/* in case we actually have a full-duplex socket
> +			 * and not two pipes; we can't use "out" because
> +			 * it has been closed already, but in the full-duplex
> +			 * case, "in" and "out" are merely dups of each other.
> +			 * We can't directly use "in" because it may be
> +			 * pointing to the sideband demuxer now */
> +			shutdown(fd[0], SHUT_WR);
>  			if (use_sideband)
>  				finish_async(&demux);
>  			return -1;
> 
> It does call shutdown() on a non-socket in the pipe case. That should be
> a harmless noop, AFAIK.

If we do care (or if we just want to be cleaner), this patch series also
works (and goes on top of the same deadlock topic, i.e., e07fd15):

  [1/3]: connect: treat generic proxy processes like ssh processes
  [2/3]: connect: let callers know if connection is a socket
  [3/3]: send-pack: avoid deadlock on git:// push with failed pack-objects

Another approach would be to actually spawn a pipe-based helper for tcp
connections (sort of a "git netcat"). That would mean all git-protocol
connections would get the same descriptor semantics, in case any other
bugs are lurking. I'm not sure if the ugliness (extra process to manage)
and decreased efficiency (pointlessly proxying data through an extra set
of pipes) are worth it. The only thing which makes me not reject it out
of hand is that it is already how git-over-ssh works (and not unlike
git-over-http), so the extra process and inefficiency are probably not
_that_ big a deal. It just feels ugly. I wish there were a portable way
to split a full-duplex socket into two half-duplex halves, but AFAIK,
that is not possible.

-Peff