Re: [PATCHv3 2/9] send-pack: Attempt to retrieve remote status even if pack-objects fails
From: Jeff King <hidden>
Date: 2016-06-15 22:51:15
On Mon, May 16, 2011 at 02:13:54AM -0400, Jeff King wrote:
quoted hunk ↗ jump to hunk
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