From: Jeff King <peff@peff.net>
To: Johannes Sixt <j6t@kdbg.org>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH/RFC 1/2] send-pack --stateless-rpc: properly close the outgoing channel
Date: Mon, 25 Apr 2011 12:40:38 -0400 [thread overview]
Message-ID: <20110425164038.GA1589@sigill.intra.peff.net> (raw)
In-Reply-To: <4DB48B2C.2090904@kdbg.org>
On Sun, Apr 24, 2011 at 10:42:20PM +0200, Johannes Sixt wrote:
> 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.
>
> [...]
>
> 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.
Note that we can also call send_pack directly from git-push via the
transport.c interface. I didn't check whether one can actually trigger
stateless-rpc this way, though; it looks like git-remote-http ends up
exec'ing a separate 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.
I was able to get a hang using v1.7.5 compiled with pthreads. You need
to have a server accepting smart-http push (if you use github, you can
create an empty test repo there, which is sufficient).
And then do a modified version of the test I posted earlier:
UPSTREAM=https://peff@github.com/peff/test.git
git init child &&
cd child &&
git remote add origin $UPSTREAM &&
echo content >file &&
git add file &&
git commit -m one &&
echo content >>file &&
git add file &&
git commit -m two &&
sha1=`git rev-parse HEAD:file` &&
file=`echo $sha1 | sed 's,..,&/,'` &&
rm -fv .git/objects/$file
where obviously you need to tweak $UPSTREAM to point to the repo you
created. That sets up the broken repo state. You can then try "git
push" in the repo with various versions to check their behavior.
With stock v1.7.5, this will hang after pack-objects reports the fatal
error. With your patch, it exits immediately, though the output looks
like this:
$ git push
Password:
Counting objects: 5, done.
error: unable to find ea0faeb6073ff6cb085727c3647be457051e6ed7
fatal: unable to read ea0faeb6073ff6cb085727c3647be457051e6ed7
fatal: The remote end hung up unexpectedly
fatal: The remote end hung up unexpectedly
fatal: write error: Bad file descriptor
which could perhaps be a little nicer, but is probably not a big deal (I
didn't dig very deep, but presumably we should exit a little more
immediately after seeing pack-objects fail).
-Peff
next prev parent reply other threads:[~2011-04-25 16:40 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 ` [PATCH/RFC 1/2] send-pack --stateless-rpc: properly close the outgoing channel Johannes Sixt
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 ` Jeff King [this message]
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110425164038.GA1589@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j6t@kdbg.org \
/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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).