In the non-stateless-rpc case, the writable end of the channel to the remote repo is closed by the start_command() call that runs the pack-objects process (after pack-objects inherited a copy). But in the --stateless-rpc case, where send-pack takes care of writing data to the channel, this was missed. Signed-off-by: Johannes Sixt <j6t@xxxxxxxx> --- Am 14.04.2011 23:21, schrieb Junio C Hamano: > Jeff King <peff@xxxxxxxx> writes: > >> I think I am leaning a bit towards (2a). It's simple, and it's not like >> this is library code with a million unknown callers; fixing it simply >> and cleanly with a nice commit message is probably sufficient. > > This really sounds like a plan. Even if we _might_ later want to go to > 2b. or some other solution, we will know what pattern to grep for. Here's a 2-patch series that implements this plan. The patches go on top of 38a81b4e (receive-pack: Wrap status reports inside side-band-64k) just like Jeff's series (jk/maint-push-async-hang). Warning: This patch is untested. Furthermore, it does not even fix a resource leak because the fd that is now closed in pack_objects() would be closed later in cmd_send_pack. However, without closing the fd earlier like this, a --stateless-rpc invocation could theoretically dead-lock just like a regular invocation in a NO_PTHREADS build. But I also don't know how to test-drive send-pack --stateless-rpc to construct such a case. Any hints how to do that would be appreciated. -- Hannes builtin-send-pack.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/builtin-send-pack.c b/builtin-send-pack.c index 2478e18..089058b 100644 --- a/builtin-send-pack.c +++ b/builtin-send-pack.c @@ -97,6 +97,7 @@ 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)) -- 1.7.5.rc1.97.ge0653 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html