[PATCH/RFC 1/2] send-pack --stateless-rpc: properly close the outgoing channel
- From
Johannes Sixt <j6t@kdbg.org>
- Date
- Apr 24, 2011, 20:42 UTC
- Message-ID
- <4DB48B2C.2090904@kdbg.org>
- In-Reply-To
- <7vsjtkfs10.fsf@alter.siamese.dyndns.org>
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@kdbg.org> --- Am 14.04.2011 23:21, schrieb Junio C Hamano:
Show 8 quoted lines
> Jeff King <peff@peff.net> 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