git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Johan Herland <johan@herland.net>
Cc: Junio C Hamano <gitster@pobox.com>,
	Shawn Pearce <spearce@spearce.org>,
	git@vger.kernel.org, Johannes Sixt <j6t@kdbg.org>
Subject: [PATCH 3/3] send-pack: avoid deadlock on git:// push with failed pack-objects
Date: Mon, 16 May 2011 02:52:57 -0400	[thread overview]
Message-ID: <20110516065257.GC19078@sigill.intra.peff.net> (raw)
In-Reply-To: <20110516063944.GB25731@sigill.intra.peff.net>

Commit 09c9957c fixes a deadlock in which pack-objects
fails, the remote end is still waiting for pack data, and we
are still waiting for the remote end to say something (see
that commit for a much more in-depth explanation).

We solved the problem there by making sure the output pipe
is closed on error; thus the remote sees EOF, and proceeds
to complain and close its end of the connection.

However, in the special case of push over git://, we don't
have a pipe, but rather a full-duplex socket, with another
dup()-ed descriptor in place of the second half of the pipe.
In this case, closing the second descriptor signals nothing
to the remote end, and we still deadlock.

This patch calls shutdown() explicitly to signal EOF to the
other side.

Signed-off-by: Jeff King <peff@peff.net>
---
And if you wanted to drop the first two patches, this probably works OK
without the conditional, as the shutdown is just a no-op on a pipe
descriptor then.

 builtin-send-pack.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 3e70795..682a3f9 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -520,6 +520,8 @@ int send_pack(struct send_pack_args *args,
 				ref->status = REF_STATUS_NONE;
 			if (args->stateless_rpc)
 				close(out);
+			if (git_connection_is_socket(conn))
+				shutdown(fd[0], SHUT_WR);
 			if (use_sideband)
 				finish_async(&demux);
 			return -1;
-- 
1.7.5.1.13.g0ca09.dirty

  parent reply	other threads:[~2011-05-16  6:53 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-13 16:54 [PATCH 2/2] receive-pack: Add receive.denyObjectLimit to refuse push with too many objects Johan Herland
2011-05-13 17:09 ` Junio C Hamano
2011-05-14  1:43   ` Johan Herland
2011-05-14  2:03     ` [PATCHv2 2/2] receive-pack: Add receive.objectCountLimit " Johan Herland
2011-05-14  2:30       ` Shawn Pearce
2011-05-14 13:17         ` Johan Herland
2011-05-14 22:17           ` Shawn Pearce
2011-05-15 17:42             ` Johan Herland
2011-05-15 21:37               ` [PATCHv3 0/9] Push limits Johan Herland
2011-05-15 21:37                 ` [PATCHv3 1/9] Update technical docs to reflect side-band-64k capability in receive-pack Johan Herland
2011-05-15 21:37                 ` [PATCHv3 2/9] send-pack: Attempt to retrieve remote status even if pack-objects fails Johan Herland
2011-05-16  4:07                   ` Jeff King
2011-05-16  6:13                     ` Jeff King
2011-05-16  6:39                       ` Jeff King
2011-05-16  6:46                         ` [PATCH 1/3] connect: treat generic proxy processes like ssh processes Jeff King
2011-05-16 19:57                           ` Johannes Sixt
2011-05-16 23:12                             ` Junio C Hamano
2011-05-17  5:54                             ` Jeff King
2011-05-17 20:14                               ` Johannes Sixt
2011-05-18  8:57                                 ` Jeff King
2011-05-16  6:52                         ` [PATCH 2/3] connect: let callers know if connection is a socket Jeff King
2011-05-16  6:52                         ` Jeff King [this message]
2011-05-16 20:02                           ` [PATCH 3/3] send-pack: avoid deadlock on git:// push with failed pack-objects Johannes Sixt
2011-05-17  5:56                             ` Jeff King
2011-05-18 20:24                               ` [PATCH] Windows: add a wrapper for the shutdown() system call Johannes Sixt
2011-05-15 21:37                 ` [PATCHv3 3/9] pack-objects: Allow --max-pack-size to be used together with --stdout Johan Herland
2011-05-15 22:06                   ` Shawn Pearce
2011-05-16  1:39                     ` Johan Herland
2011-05-16  6:12                   ` Junio C Hamano
2011-05-16  9:27                     ` Johan Herland
2011-05-15 21:37                 ` [PATCHv3 4/9] pack-objects: Teach new option --max-object-count, similar to --max-pack-size Johan Herland
2011-05-15 22:07                   ` Shawn Pearce
2011-05-15 22:31                     ` Johan Herland
2011-05-15 23:48                       ` Shawn Pearce
2011-05-16  6:25                     ` Junio C Hamano
2011-05-16  9:49                       ` Johan Herland
2011-05-15 21:37                 ` [PATCHv3 5/9] pack-objects: Teach new option --max-commit-count, limiting #commits in pack Johan Herland
2011-05-15 21:37                 ` [PATCHv3 6/9] receive-pack: Prepare for addition of the new 'limit-*' family of capabilities Johan Herland
2011-05-16  6:50                   ` Junio C Hamano
2011-05-16  9:53                     ` Johan Herland
2011-05-16 22:02                     ` Sverre Rabbelier
2011-05-16 22:07                       ` Junio C Hamano
2011-05-16 22:09                         ` Sverre Rabbelier
2011-05-16 22:12                           ` Junio C Hamano
2011-05-16 22:16                             ` Sverre Rabbelier
2011-05-15 21:37                 ` [PATCHv3 7/9] send-pack/receive-pack: Allow server to refuse pushes with too many objects Johan Herland
2011-05-15 21:37                 ` [PATCHv3 8/9] send-pack/receive-pack: Allow server to refuse pushing too large packs Johan Herland
2011-05-15 21:37                 ` [PATCHv3 9/9] send-pack/receive-pack: Allow server to refuse pushes with too many commits Johan Herland
2011-05-15 21:52                 ` [PATCHv3 0/9] Push limits Ævar Arnfjörð Bjarmason
2011-05-14 17:50         ` [PATCHv2 2/2] receive-pack: Add receive.objectCountLimit to refuse push with too many objects Junio C Hamano
2011-05-14 22:27           ` Shawn Pearce
2011-05-13 18:20 ` [PATCH 2/2] receive-pack: Add receive.denyObjectLimit " Johannes Sixt
2011-05-14  1:49   ` Johan Herland

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110516065257.GC19078@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=johan@herland.net \
    --cc=spearce@spearce.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).