Re: [PATCH 4/4] send-pack: abort sideband demuxer on pack-objects error

From: Johannes Sixt <j6t@kdbg.org>
Date: Apr 13, 2011, 19:53 UTC
Message-ID: <201104132153.06429.j6t@kdbg.org>
In-Reply-To: <20110331184443.GD16906@sigill.intra.peff.net>

On Donnerstag, 31. März 2011, Jeff King wrote:

Show 15 quoted lines

> If pack-objects dies prematurely (for example, because there
> is some repo corruption), we are careful to clean up the
> sideband demuxer (if it is being used) with finish_async.
> However, for an async implementation which forks (i.e., not
> Windows), that means we will waitpid() for the async
> process.
>
> Meanwhile, the async sideband demuxer will continue trying
> to stream data from the remote repo until it gets EOF.
> Depending on what data pack-objects actually sent, the
> remote repo may not actually send anything (e.g., if we sent
> nothing and it is simply waiting for the pack). This leads
> to deadlock cycle in which send-pack waits on the demuxer,
> the demuxer waits on the remote receive-pack, and the remote
> receive-pack waits on send-pack to send the pack data.

This is an indication that a writable end of the pipe between send-pack and receive-pack is still open. This fixes the deadlock for me without having to kill the demuxer explicitly:

diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 5e772c7..db32ded 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -229,6 +229,9 @@ static void print_helper_status(struct ref *ref)
 static int sideband_demux(int in, int out, void *data)
 {
 	int *fd = data;
+#ifdef NO_PTHREADS
+	close(fd[1]);
+#endif
 	int ret = recv_sideband("send-pack", fd[0], out);
 	close(out);
 	return ret;

If only I had a brilliant idea how to forge this into a re-usable pattern...

-- Hannes