From: Johannes Sixt <j6t@kdbg.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jeff King <peff@peff.net>, git@vger.kernel.org
Subject: [PATCH/RFC 1/2] send-pack --stateless-rpc: properly close the outgoing channel
Date: Sun, 24 Apr 2011 22:42:20 +0200 [thread overview]
Message-ID: <4DB48B2C.2090904@kdbg.org> (raw)
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:
> 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
next prev parent reply other threads:[~2011-04-24 20:42 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-03-31 18:42 [PATCH 0/4] fix hang in git push when pack-objects fails Jeff King
2011-03-31 18:43 ` [PATCH 1/4] teach wait_or_whine a "quiet" mode Jeff King
2011-03-31 20:56 ` Johannes Sixt
2011-04-01 1:35 ` Jeff King
2011-03-31 18:44 ` [PATCH 2/4] finish_async: be quiet when waiting for async process Jeff King
2011-03-31 18:44 ` [PATCH 3/4] run-command: allow aborting async code prematurely Jeff King
2011-04-01 9:36 ` Erik Faye-Lund
2011-04-01 13:59 ` Jeff King
2011-03-31 18:44 ` [PATCH 4/4] send-pack: abort sideband demuxer on pack-objects error Jeff King
2011-04-13 19:53 ` Johannes Sixt
2011-04-14 13:54 ` Jeff King
2011-04-14 19:36 ` Johannes Sixt
2011-04-14 20:21 ` Jeff King
2011-04-14 20:43 ` Johannes Sixt
2011-04-14 20:51 ` Jeff King
2011-04-14 21:05 ` Johannes Sixt
2011-04-14 21:21 ` Junio C Hamano
2011-04-24 20:42 ` Johannes Sixt [this message]
2011-04-24 20:49 ` [PATCH 2/2] send-pack: avoid deadlock when pack-object dies early Johannes Sixt
2011-04-25 16:50 ` Jeff King
2011-04-25 17:41 ` Johannes Sixt
2011-04-25 17:51 ` Junio C Hamano
2011-04-25 21:04 ` [PATCH v2] " Johannes Sixt
2011-04-26 8:23 ` Jeff King
2011-04-25 16:40 ` [PATCH/RFC 1/2] send-pack --stateless-rpc: properly close the outgoing channel Jeff King
2011-03-31 18:45 ` [PATCH 5/4] run-command: implement abort_async for pthreads Jeff King
2011-04-01 9:41 ` Erik Faye-Lund
2011-04-01 10:15 ` Erik Faye-Lund
2011-04-01 17:27 ` Johannes Sixt
2011-04-01 17:38 ` Jeff King
2011-04-01 19:26 ` Erik Faye-Lund
2011-04-01 19:33 ` Erik Faye-Lund
2011-04-01 19:42 ` Johannes Sixt
2011-04-01 19:57 ` Erik Faye-Lund
2011-04-01 20:05 ` Jeff King
2011-04-01 20:13 ` Erik Faye-Lund
2011-04-01 20:17 ` Jeff King
2011-04-01 20:18 ` Jeff King
2011-04-01 20:34 ` Erik Faye-Lund
2011-04-01 20:36 ` Johannes Sixt
2011-04-01 20:41 ` Erik Faye-Lund
2011-04-01 20:18 ` Johannes Sixt
2011-04-01 20:31 ` Erik Faye-Lund
2011-04-01 21:16 ` Jeff King
2011-04-02 12:27 ` Erik Faye-Lund
2011-04-01 14:00 ` Jeff King
2011-03-31 20:45 ` [PATCH 0/4] fix hang in git push when pack-objects fails Johannes Sixt
2011-04-01 1:34 ` Jeff King
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=4DB48B2C.2090904@kdbg.org \
--to=j6t@kdbg.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.