{"thread":{"id":"52227","subject":"git name-rev looks at refs/notes, refs/svn/map: stack overflow","startedAt":"2019-11-09T11:31:47Z","lastAt":"2019-11-12T18:34:02Z","messageCount":4,"participants":["Sebastiaan Dammann","Jeff King","SZEDER Gábor"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"385832","messageId":"CAE7Eq9jJzftkP9JWFpstS96SiCd+jO_adSQ-HruyYYNi3gWe7w@mail.gmail.com","threadId":"52227","inReplyTo":null,"subject":"git name-rev looks at refs/notes, refs/svn/map: stack overflow","fromName":"Sebastiaan Dammann","fromEmail":"triadsebas@gmail.com","sentAt":"2019-11-09T11:31:31Z","receivedAt":"2019-11-09T11:31:47Z","isPatch":false,"sender":{"key":"triadsebas@gmail.com","avatar":"https://gravatar.com/avatar/7d735a0a9744473962da8c6538a6950ceaa410e84476c8733d9d0863a442c772?d=mp&s=160"},"body":"Hello git users,\n\nI'm running into git crashing with an stack overflow and hope to hear\nyour views on this.\n\nI have a from SVN converted repository (using Subgit). If I execute\n\"git name-rev 92f054ba5c71b8e689aed529ff31370984089b64\" on this bare\nrepository, git crashes with a stack overflow.\n\nOne property of the git conversion process is that at every commit a\nnote is attached which lists the SVN version number. This repository\nhas over 50000 commits.\nIn addition, refs exist at refs/svn/map which also contains many\nentries. It appears the name-rev looks at the notes and other refs as\nwell.\n\nThe backtrace of the stack overflow is this:\n\n[repeats many times]\n#12976 0x000000000046d178 in name_rev (commit=0x37241b0,\n tip_name=0x37ccb70 \"notes/commits\", taggerdate=1572995101, generation=3,\n distance=3, from_tag=0, deref=0) at builtin/name-rev.c:139\n#12977 0x000000000046d178 in name_rev (commit=0x3724160,\n tip_name=0x37ccb70 \"notes/commits\", taggerdate=1572995101, generation=2,\n distance=2, from_tag=0, deref=0) at builtin/name-rev.c:139\n#12978 0x000000000046d178 in name_rev (commit=0x3724110,\n tip_name=0x37ccb70 \"notes/commits\", taggerdate=1572995101, generation=1,\n distance=1, from_tag=0, deref=0) at builtin/name-rev.c:139\n#12979 0x000000000046d178 in name_rev (commit=0x37240c0,\n tip_name=0x37ccb70 \"notes/commits\", taggerdate=1572995101, generation=0,\n distance=0, from_tag=0, deref=0) at builtin/name-rev.c:139\n#12980 0x000000000046d69d in name_ref (path=0x37232e5 \"notes/commits\",\n oid=0x37c6030, flags=66, cb_data=0x155ef20) at builtin/name-rev.c:277\n#12981 0x00000000005c6645 in do_for_each_ref_helper (r=0x7acee0 ,\n refname=0x37232e0 \"refs/notes/commits\", oid=0x37c6030, flags=66,\n cb_data=0x155ede0) at refs.c:1552\n#12982 0x00000000005cfcd1 in do_for_each_repo_ref_iterator (\n r=0x7acee0 , iter=0x37c6190,\n fn=0x5c65ff , cb_data=0x155ede0)\n at refs/iterator.c:418\n#12983 0x00000000005c66de in do_for_each_ref (refs=0x3716aa8,\n prefix=0x71d34f <__ac_HASH_UPPER+271> \"\", fn=0x46d3ff , trim=0,\n flags=0, cb_data=0x155ef20) at refs.c:1566\n#12984 0x00000000005c673f in refs_for_each_ref (refs=0x3716aa8,\n fn=0x46d3ff , cb_data=0x155ef20) at refs.c:1572\n#12985 0x00000000005c677a in for_each_ref (fn=0x46d3ff ,\n cb_data=0x155ef20) at refs.c:1577\n#12986 0x000000000046e2c1 in cmd_name_rev (argc=0, argv=0x37106b8, prefix=0x0)\n at builtin/name-rev.c:495\n#12987 0x000000000040305c in run_builtin (p=0x6b56e8 , argc=2,\n argv=0x37106b0) at git.c:444\n#12988 0x0000000000403446 in handle_builtin (argc=2, argv=0x37106b0)\n at git.c:673\n#12989 0x00000000004036fc in run_argv (argcp=0x155fdb0, argv=0x155fd58)\n at git.c:740\n#12990 0x0000000000403b62 in cmd_main (argc=2, argv=0x37106b0) at git.c:871\n#12991 0x00000000004c8647 in main (argc=3, argv=0x37106a8) at common-main.c:52\n\nIf I trace it back to the source we get this:\n(gdb)\n#16 0x000000000046d178 in name_rev (commit=0x3b948d30,\n tip_name=0x37ccb70 \"notes/commits\", taggerdate=1572995101,\n generation=12963, distance=12963, from_tag=0, deref=0)\n at builtin/name-rev.c:139\n139 name_rev(parents->item, tip_name, taggerdate,\n(gdb)\n#17 0x000000000046d178 in name_rev (commit=0x3b948ce0,\n tip_name=0x37ccb70 \"notes/commits\", taggerdate=1572995101,\n generation=12962, distance=12962, from_tag=0, deref=0)\n at builtin/name-rev.c:139\n139 name_rev(parents->item, tip_name, taggerdate,\n(gdb)\n#18 0x000000000046d178 in name_rev (commit=0x3b948c90,\n tip_name=0x37ccb70 \"notes/commits\", taggerdate=1572995101,\n generation=12961, distance=12961, from_tag=0, deref=0)\n at builtin/name-rev.c:139\n139 name_rev(parents->item, tip_name, taggerdate,\n(gdb)\n#19 0x000000000046d178 in name_rev (commit=0x3b948c40,\n tip_name=0x37ccb70 \"notes/commits\", taggerdate=1572995101,\n generation=12960, distance=12960, from_tag=0, deref=0)\n at builtin/name-rev.c:139\n139 name_rev(parents->item, tip_name, taggerdate,\n(gdb)\n#20 0x000000000046d178 in name_rev (commit=0x3b948bf0,\n tip_name=0x37ccb70 \"notes/commits\", taggerdate=1572995101,\n generation=12959, distance=12959, from_tag=0, deref=0)\n at builtin/name-rev.c:139\n139 name_rev(parents->item, tip_name, taggerdate,\n(gdb)\n#21 0x000000000046d178 in name_rev (commit=0x3b948ba0,\n tip_name=0x37ccb70 \"notes/commits\", taggerdate=1572995101,\n generation=12958, distance=12958, from_tag=0, deref=0)\n at builtin/name-rev.c:139\n139\n\nEssentially too much, but not endless recursion, causing a stack overflow.\n\nThe destructive workaround at the moment is to delete the refs, then\nrun an aggressive gc:\ngit update-ref -d refs/notes/commits\ngit update-ref -d refs/svn/map\ngit gc --prune=all --aggressive\n\nI was redirected here by the git-for-windows team (issue link if\nyou're interested:\nhttps://github.com/git-for-windows/git/issues/2393), so I'm going to\nrepeat what was said there:\n\n> But yes, the problem seems to be the many, many notes. Which points to a deeper issue: name_rev should not even look at anything byt refs/heads/*, refs/remotes/* and refs/tags/. Certainly it should exclude refs/notes/*...\n> This, however, is not Windows-specific. Maybe I can ask you to report this issue to the Git mailing list (git@vger.kernel.org, please suppress all HTML lest the mail will be bounced)?\n\nI hope to hear your view on this. Is this an (confirmed) issue with\ngit? Are there beside the workaround I mentioned, any other\nworkarounds?\n\nGroeten, Kind regards,\nSebastiaan Dammann\n"},{"id":"385872","messageId":"20191111042149.GE6379@sigill.intra.peff.net","threadId":"52227","inReplyTo":"CAE7Eq9jJzftkP9JWFpstS96SiCd+jO_adSQ-HruyYYNi3gWe7w@mail.gmail.com","subject":"Re: git name-rev looks at refs/notes, refs/svn/map: stack overflow","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-11-11T04:21:49Z","receivedAt":"2019-11-11T04:21:52Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Nov 09, 2019 at 12:31:31PM +0100, Sebastiaan Dammann wrote:\n\n> The destructive workaround at the moment is to delete the refs, then\n> run an aggressive gc:\n> git update-ref -d refs/notes/commits\n> git update-ref -d refs/svn/map\n> git gc --prune=all --aggressive\n\nIf the issue is just traversing from those refs, you shouldn't need to\ndo a \"gc\". In fact, you should be able to use the \"--refs\" option to\nname-rev to avoid them without deleting them.\n\nAll of that's obviously a workaround, though. The real issue is the\nstack exhaustion.\n\n> I hope to hear your view on this. Is this an (confirmed) issue with\n> git? Are there beside the workaround I mentioned, any other\n> workarounds?\n\nYes, this is well known. It's covered in the test suite (unfortunately\nstill failing, of course) since 31625b34c0 (t6120: test describe and\nname-rev with deep repos, 2017-09-07).\n\nThere was a proposed fix recently in:\n\n https://public-inbox.org/git/20190919214712.7348-1-szeder.dev@gmail.com/\n\nbut it doesn't seem to have been picked up. I'm not sure what the\ncurrent status is.\n\n-Peff\n"},{"id":"385906","messageId":"20191111140956.GJ4348@szeder.dev","threadId":"52227","inReplyTo":"20191111042149.GE6379@sigill.intra.peff.net","subject":"Re: git name-rev looks at refs/notes, refs/svn/map: stack overflow","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-11-11T14:09:56Z","receivedAt":"2019-11-11T14:10:02Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Sun, Nov 10, 2019 at 11:21:49PM -0500, Jeff King wrote:\n> On Sat, Nov 09, 2019 at 12:31:31PM +0100, Sebastiaan Dammann wrote:\n\n> All of that's obviously a workaround, though. The real issue is the\n> stack exhaustion.\n> \n> > I hope to hear your view on this. Is this an (confirmed) issue with\n> > git? Are there beside the workaround I mentioned, any other\n> > workarounds?\n> \n> Yes, this is well known. It's covered in the test suite (unfortunately\n> still failing, of course) since 31625b34c0 (t6120: test describe and\n> name-rev with deep repos, 2017-09-07).\n> \n> There was a proposed fix recently in:\n> \n> https://public-inbox.org/git/20190919214712.7348-1-szeder.dev@gmail.com/\n> \n> but it doesn't seem to have been picked up. I'm not sure what the\n> current status is.\n\nIt's getting along, being polished, clarified and fine-tuned ever so\nslowly.\n\nE.g. it turned out that the performance penalty from eliminating the\nrecursion is basically entirely caused by using a 'commit-list' to\nstore interesting parents (the overhead of a malloc() on each insert\nand a free() on each pop). Switching to a LIFO 'prio-queue' doesn't\ncause any measurable slowdown.\n\n"},{"id":"386028","messageId":"CAE7Eq9hEiVf1rMNdWx55_nQsz2gVv0N+s1KckK1evtmruqcHyA@mail.gmail.com","threadId":"52227","inReplyTo":"20191111140956.GJ4348@szeder.dev","subject":"Re: git name-rev looks at refs/notes, refs/svn/map: stack overflow","fromName":"Sebastiaan Dammann","fromEmail":"triadsebas@gmail.com","sentAt":"2019-11-12T18:33:49Z","receivedAt":"2019-11-12T18:34:02Z","isPatch":false,"sender":{"key":"triadsebas@gmail.com","avatar":"https://gravatar.com/avatar/7d735a0a9744473962da8c6538a6950ceaa410e84476c8733d9d0863a442c772?d=mp&s=160"},"body":"Is there anything I can do to help? I have real-life (though\nproprietary) repositories to test the fix on.\n\nOn Mon, 11 Nov 2019 at 15:09, SZEDER Gábor wrote:\n>\n> On Sun, Nov 10, 2019 at 11:21:49PM -0500, Jeff King wrote:\n> > On Sat, Nov 09, 2019 at 12:31:31PM +0100, Sebastiaan Dammann wrote:\n>\n> > All of that's obviously a workaround, though. The real issue is the\n> > stack exhaustion.\n> >\n> > > I hope to hear your view on this. Is this an (confirmed) issue with\n> > > git? Are there beside the workaround I mentioned, any other\n> > > workarounds?\n> >\n> > Yes, this is well known. It's covered in the test suite (unfortunately\n> > still failing, of course) since 31625b34c0 (t6120: test describe and\n> > name-rev with deep repos, 2017-09-07).\n> >\n> > There was a proposed fix recently in:\n> >\n> > https://public-inbox.org/git/20190919214712.7348-1-szeder.dev@gmail.com/\n> >\n> > but it doesn't seem to have been picked up. I'm not sure what the\n> > current status is.\n>\n> It's getting along, being polished, clarified and fine-tuned ever so\n> slowly.\n>\n> E.g. it turned out that the performance penalty from eliminating the\n> recursion is basically entirely caused by using a 'commit-list' to\n> store interesting parents (the overhead of a malloc() on each insert\n> and a free() on each pop). Switching to a LIFO 'prio-queue' doesn't\n> cause any measurable slowdown.\n>\n"}]}