From e07fd15b1295393ddf954b9fdd0c3a961e5509c5 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 5 May 2011 02:18:45 -0400 Subject: [PATCH] send-pack: unbreak push over stateless rpc Commit 09c9957 (send-pack: avoid deadlock when pack-object dies early, 2011-04-25) attempted to fix a hang in the stateless rpc case by closing a file descriptor early, but we still need that descriptor. Basically the deadlock can happen when pack-objects fails, and the descriptor to upstream is left open. We never send the pack, so the upstream is left waiting for us to say something, and we are left waiting for upstream to close the connection. In the non-rpc case, our descriptor points straight to the upstream. We hand it off to run-command, which takes ownership and closes the descriptor after pack-objects finishes (whether it succeeds or not). Commit 09c9957 tried to emulate that in the rpc case. That isn't right, though. We actually have a descriptor going back to the remote-helper, and we need to keep using it after pack-objects is finished. Closing it early completely breaks pushing via smart-http. We still need to do something on error to signal the remote-helper that we won't be sending any pack data (otherwise we get the deadlock). In an ideal world, we would send a special packet back that says "Sorry, there was an error". But the remote-helper doesn't understand any such packet, so the best we can do is close the descriptor and let it report that we hung up unexpectedly. Signed-off-by: Jeff King Acked-by: Johannes Sixt Signed-off-by: Junio C Hamano --- builtin-send-pack.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/builtin-send-pack.c b/builtin-send-pack.c index 6516288711..3e70795ef9 100644 --- a/builtin-send-pack.c +++ b/builtin-send-pack.c @@ -97,7 +97,6 @@ static int pack_objects(int fd, struct ref *refs, struct extra_have_objects *ext free(buf); close(po.out); po.out = -1; - close(fd); } if (finish_command(&po)) @@ -519,6 +518,8 @@ int send_pack(struct send_pack_args *args, if (pack_objects(out, remote_refs, extra_have, args) < 0) { for (ref = remote_refs; ref; ref = ref->next) ref->status = REF_STATUS_NONE; + if (args->stateless_rpc) + close(out); if (use_sideband) finish_async(&demux); return -1;