git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "René Scharfe" <rene.scharfe@lsrfire.ath.cx>
To: Jean-Jacques Lafay <jeanjacques.lafay@gmail.com>
Cc: msysgit@googlegroups.com, Git List <git@vger.kernel.org>,
	Philip Oakley <philipoakley@iee.org>, Jeff King <peff@peff.net>
Subject: Re: [PATCH] git tag --contains : avoid stack overflow
Date: Sun, 11 Nov 2012 17:46:32 +0100	[thread overview]
Message-ID: <509FD668.20609@lsrfire.ath.cx> (raw)
In-Reply-To: <d9843c09-3ca9-406e-9df8-78603235d985@googlegroups.com>

Am 10.11.2012 22:13, schrieb Jean-Jacques Lafay:
> Le samedi 10 novembre 2012 21:00:10 UTC+1, Philip Oakley a écrit :
>
>     From: "Jean-Jacques Lafay" <jeanjacq...@gmail.com <javascript:>>
>     Sent: Saturday,
>     November 10, 2012 5:36 PM
>      > In large repos, the recursion implementation of contains(commit,
>      > commit_list)
>      > may result in a stack overflow. Replace the recursion with a loop to
>      > fix it.
>      >
>      > Signed-off-by: Jean-Jacques Lafay <jeanjacq...@gmail.com
>     <javascript:>>
>
>     This is a change to the main git code so it is better to submit it to
>     the main git list at g...@vger.kernel.org <javascript:> (plain text,
>     no HTML, first
>     post usually has a delay ;-)
>
>     It sounds reasonable but others may have opinions and comments.
>
>     Have you actually suffered from the stack overflow issue? You only
>     suggest it as a possibility, rather than a real problem.
>
>     Philip
>
>
> Yes, it actually crashed on me, and since the call is made by
> GitExtension while browsing the commit history, it was quickly annoying
> to have windows popping a "git.exe stopped working" message box at my
> face every time I clicked on a line of the history ;-)  (well, you can
> sort of work around it by having the "file tree" or "diff" tab active)
>
> Coincidentally, as I was having a glance at the recent topics, I saw
> that someone else experienced it.
>
> However, I couldn't reproduce it on Linux : where the windows
> implementations crashes at a ~32000 depth (*not* exactly 32768, mind
> you), on linux it happily went through 100000 commits. I didn't take
> time to look much further, but maybe on my 64 bit Linux VM, the process
> can afford to reserve a much bigger address range for the stack of each
> thread than the 1Mb given to 32 bit processes on windows.
> Jean-Jacques.

I can reproduce it on Linux (Debian testing amd64) with ulimit -s 1000 
to reduce the stack size from its default value of 8MB.

After reverting ffc4b8012d9a4f92ef238ff72c0d15e9e1b402ed (tag: speed up 
--contains calculation) the test passes even with the smaller stack, but 
it makes "git tag --contains" take thrice the time as before.

René

  parent reply	other threads:[~2012-11-11 16:46 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1352568970-4669-1-git-send-email-jeanjacques.lafay@gmail.com>
2012-11-10 20:00 ` [PATCH] git tag --contains : avoid stack overflow Philip Oakley
2012-11-10 21:13   ` Jean-Jacques Lafay
2012-11-10 21:33     ` Pat Thoyts
2012-11-11 16:46     ` René Scharfe [this message]
2012-11-11 16:54       ` Jeff King
2012-11-11 23:10         ` Johannes Schindelin
2012-11-12 22:27         ` Jean-Jacques Lafay
2012-11-12 23:14           ` Jeff King
2012-11-13  1:16             ` Johannes Schindelin
2012-11-13  3:46               ` [msysGit] " Jeff King
2012-11-13  4:01                 ` Johannes Schindelin
2012-11-13  4:05                   ` Jeff King
2012-11-13  4:52                     ` Johannes Schindelin
2012-11-13  4:51                 ` Junio C Hamano
2012-11-13  6:08                   ` Jeff King
2014-04-16 14:15 Stepan Kasal
2014-04-16 15:46 ` Jeff King
2014-04-17 17:31   ` Johannes Schindelin
2014-04-17 21:32     ` Jeff King
2014-04-17 21:52       ` Johannes Schindelin
2014-04-17 21:58         ` 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=509FD668.20609@lsrfire.ath.cx \
    --to=rene.scharfe@lsrfire.ath.cx \
    --cc=git@vger.kernel.org \
    --cc=jeanjacques.lafay@gmail.com \
    --cc=msysgit@googlegroups.com \
    --cc=peff@peff.net \
    --cc=philipoakley@iee.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.
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 NNTP newsgroup(s).