{"thread":{"id":"21596","subject":"Git in next is broken","startedAt":"2009-11-12T20:36:50Z","lastAt":"2009-11-13T21:25:56Z","messageCount":8,"participants":["Nicolas Pitre","René Scharfe","Julian Phillips","Eric Raible"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"127473","messageId":"alpine.LFD.2.00.0911121513470.16711@xanadu.home","threadId":"21596","inReplyTo":null,"subject":"Git in next is broken","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2009-11-12T20:36:50Z","receivedAt":"2009-11-12T20:36:50Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"Simply issuing a \"git fetch\" in my copy of git.git makes glibc complain \nwith this:\n\n*** glibc detected *** git: corrupted double-linked list: 0x0000000000974180 ***\n\nThe gdb backtrace is:\n\n(gdb) bt\n#0 0x0000003c76632f05 in raise () from /lib64/libc.so.6\n#1 0x0000003c76634a73 in abort () from /lib64/libc.so.6\n#2 0x0000003c76672438 in __libc_message () from /lib64/libc.so.6\n#3 0x0000003c76677ec8 in malloc_printerr () from /lib64/libc.so.6\n#4 0x0000003c7667a23e in _int_free () from /lib64/libc.so.6\n#5 0x0000003c7667a486 in free () from /lib64/libc.so.6\n#6 0x0000000000493f3f in ref_remove_duplicates (ref_map=0x7562b0)\n at remote.c:756\n#7 0x0000000000424afc in get_ref_map () at builtin-fetch.c:165\n#8 do_fetch () at builtin-fetch.c:644\n#9 cmd_fetch (argc=, argv=0x7fffffffe6a0,\n prefix=) at builtin-fetch.c:754\n#10 0x0000000000403d83 in run_builtin () at git.c:251\n#11 handle_internal_command (argc=1, argv=0x7fffffffe6a0) at git.c:396\n#12 0x0000000000403f2d in run_argv () at git.c:438\n#13 main (argc=1, argv=0x7fffffffe6a0) at git.c:509\n\nBisection reveals the following culprit:\n\ncommit 73cf0822b2a4ffa7ad559d1f0772e39718fc7776\nAuthor: Julian Phillips \nDate: Sun Oct 25 21:28:11 2009 +0000\n\n remote: Make ref_remove_duplicates faster for large numbers of refs\n\n The ref_remove_duplicates function was very slow at dealing with very\n large numbers of refs. This is because it was using a linear search\n through all remaining refs to find any duplicates of the current ref.\n\n Rewriting it to use a string list to keep track of which refs have\n already been seen and removing duplicates when they are found is much\n more efficient.\n\n Signed-off-by: Julian Phillips \n Signed-off-by: Junio C Hamano \n\nReverting that commit from next does indeed fix the problem.\nNote that this problem doesn't show up with all repositoryes.\n\n\nNicolas\n"},{"id":"127476","messageId":"4AFC8960.9090808@lsrfire.ath.cx","threadId":"21596","inReplyTo":"alpine.LFD.2.00.0911121513470.16711@xanadu.home","subject":"Re: Git in next is broken","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2009-11-12T22:17:04Z","receivedAt":"2009-11-12T22:17:04Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Nicolas Pitre schrieb:\n> Simply issuing a \"git fetch\" in my copy of git.git makes glibc complain \n> with this:\n> \n> *** glibc detected *** git: corrupted double-linked list: 0x0000000000974180 ***\n> \n> The gdb backtrace is:\n> \n> (gdb) bt\n> #0 0x0000003c76632f05 in raise () from /lib64/libc.so.6\n> #1 0x0000003c76634a73 in abort () from /lib64/libc.so.6\n> #2 0x0000003c76672438 in __libc_message () from /lib64/libc.so.6\n> #3 0x0000003c76677ec8 in malloc_printerr () from /lib64/libc.so.6\n> #4 0x0000003c7667a23e in _int_free () from /lib64/libc.so.6\n> #5 0x0000003c7667a486 in free () from /lib64/libc.so.6\n> #6 0x0000000000493f3f in ref_remove_duplicates (ref_map=0x7562b0)\n> at remote.c:756\n> #7 0x0000000000424afc in get_ref_map () at builtin-fetch.c:165\n> #8 do_fetch () at builtin-fetch.c:644\n> #9 cmd_fetch (argc=, argv=0x7fffffffe6a0,\n> prefix=) at builtin-fetch.c:754\n> #10 0x0000000000403d83 in run_builtin () at git.c:251\n> #11 handle_internal_command (argc=1, argv=0x7fffffffe6a0) at git.c:396\n> #12 0x0000000000403f2d in run_argv () at git.c:438\n> #13 main (argc=1, argv=0x7fffffffe6a0) at git.c:509\n> \n> Bisection reveals the following culprit:\n> \n> commit 73cf0822b2a4ffa7ad559d1f0772e39718fc7776\n> Author: Julian Phillips \n> Date: Sun Oct 25 21:28:11 2009 +0000\n> \n> remote: Make ref_remove_duplicates faster for large numbers of refs\n\nCan't reproduce because I don't know how to create duplicate refs, but does\nthe following help?\n\n\n remote.c | 2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 4f9f0cc..10cc985 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -754,6 +754,8 @@ void ref_remove_duplicates(struct ref *ref_map)\n \t\t\tprev->next = ref_map->next;\n \t\t\tfree(ref_map->peer_ref);\n \t\t\tfree(ref_map);\n+\t\t\tref_map = next;\n+\t\t\tcontinue;\n \t\t}\n \n \t\titem = string_list_insert(ref_map->peer_ref->name, &refs);\n"},{"id":"127477","messageId":"alpine.LNX.2.00.0911122239150.6967@reaper.quantumfyre.co.uk","threadId":"21596","inReplyTo":"4AFC8960.9090808@lsrfire.ath.cx","subject":"Re: Git in next is broken","fromName":"Julian Phillips","fromEmail":"julian@quantumfyre.co.uk","sentAt":"2009-11-12T22:45:37Z","receivedAt":"2009-11-12T22:45:37Z","isPatch":false,"sender":{"key":"julian@quantumfyre.co.uk","avatar":"https://avatars.githubusercontent.com/u/948888?v=4"},"body":"On Thu, 12 Nov 2009, Ren? Scharfe wrote:\n\n> Nicolas Pitre schrieb:\n>> Simply issuing a \"git fetch\" in my copy of git.git makes glibc complain\n>> with this:\n>>\n>> *** glibc detected *** git: corrupted double-linked list: 0x0000000000974180 ***\n>>\n>> The gdb backtrace is:\n>>\n>> (gdb) bt\n>> #0 0x0000003c76632f05 in raise () from /lib64/libc.so.6\n>> #1 0x0000003c76634a73 in abort () from /lib64/libc.so.6\n>> #2 0x0000003c76672438 in __libc_message () from /lib64/libc.so.6\n>> #3 0x0000003c76677ec8 in malloc_printerr () from /lib64/libc.so.6\n>> #4 0x0000003c7667a23e in _int_free () from /lib64/libc.so.6\n>> #5 0x0000003c7667a486 in free () from /lib64/libc.so.6\n>> #6 0x0000000000493f3f in ref_remove_duplicates (ref_map=0x7562b0)\n>> at remote.c:756\n>> #7 0x0000000000424afc in get_ref_map () at builtin-fetch.c:165\n>> #8 do_fetch () at builtin-fetch.c:644\n>> #9 cmd_fetch (argc=, argv=0x7fffffffe6a0,\n>> prefix=) at builtin-fetch.c:754\n>> #10 0x0000000000403d83 in run_builtin () at git.c:251\n>> #11 handle_internal_command (argc=1, argv=0x7fffffffe6a0) at git.c:396\n>> #12 0x0000000000403f2d in run_argv () at git.c:438\n>> #13 main (argc=1, argv=0x7fffffffe6a0) at git.c:509\n>>\n>> Bisection reveals the following culprit:\n>>\n>> commit 73cf0822b2a4ffa7ad559d1f0772e39718fc7776\n>> Author: Julian Phillips \n>> Date: Sun Oct 25 21:28:11 2009 +0000\n>>\n>> remote: Make ref_remove_duplicates faster for large numbers of refs\n>\n> Can't reproduce because I don't know how to create duplicate refs, but does\n> the following help?\n>\n>\n> remote.c | 2 ++\n> 1 files changed, 2 insertions(+), 0 deletions(-)\n>\n> diff --git a/remote.c b/remote.c\n> index 4f9f0cc..10cc985 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -754,6 +754,8 @@ void ref_remove_duplicates(struct ref *ref_map)\n> \t\t\tprev->next = ref_map->next;\n> \t\t\tfree(ref_map->peer_ref);\n> \t\t\tfree(ref_map);\n> +\t\t\tref_map = next;\n\nYou don't need this line (this is taken care of in the for(...)).\n\n> +\t\t\tcontinue;\n\nAck. This one however, you do need. Good catch.\n\n> \t\t}\n>\n> \t\titem = string_list_insert(ref_map->peer_ref->name, &refs);\n>\n\n-- \nJulian\n\n ---\nPunishment becomes ineffective after a certain point. Men become insensitive.\n \t\t-- Eneg, \"Patterns of Force\", stardate 2534.7\n"},{"id":"127489","messageId":"alpine.LFD.2.00.0911122345450.16711@xanadu.home","threadId":"21596","inReplyTo":"alpine.LNX.2.00.0911122239150.6967@reaper.quantumfyre.co.uk","subject":"Re: Git in next is broken","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2009-11-13T04:50:22Z","receivedAt":"2009-11-13T04:50:22Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 12 Nov 2009, Julian Phillips wrote:\n\n> On Thu, 12 Nov 2009, Ren? Scharfe wrote:\n> \n> > Nicolas Pitre schrieb:\n> > > Simply issuing a \"git fetch\" in my copy of git.git makes glibc complain\n> > > with this:\n> > > \n> > > *** glibc detected *** git: corrupted double-linked list:\n> > > 0x0000000000974180 ***\n> > > \n> > > The gdb backtrace is:\n> > > \n> > > (gdb) bt\n> > > #0 0x0000003c76632f05 in raise () from /lib64/libc.so.6\n> > > #1 0x0000003c76634a73 in abort () from /lib64/libc.so.6\n> > > #2 0x0000003c76672438 in __libc_message () from /lib64/libc.so.6\n> > > #3 0x0000003c76677ec8 in malloc_printerr () from /lib64/libc.so.6\n> > > #4 0x0000003c7667a23e in _int_free () from /lib64/libc.so.6\n> > > #5 0x0000003c7667a486 in free () from /lib64/libc.so.6\n> > > #6 0x0000000000493f3f in ref_remove_duplicates (ref_map=0x7562b0)\n> > > at remote.c:756\n> > > #7 0x0000000000424afc in get_ref_map () at builtin-fetch.c:165\n> > > #8 do_fetch () at builtin-fetch.c:644\n> > > #9 cmd_fetch (argc=, argv=0x7fffffffe6a0,\n> > > prefix=) at builtin-fetch.c:754\n> > > #10 0x0000000000403d83 in run_builtin () at git.c:251\n> > > #11 handle_internal_command (argc=1, argv=0x7fffffffe6a0) at git.c:396\n> > > #12 0x0000000000403f2d in run_argv () at git.c:438\n> > > #13 main (argc=1, argv=0x7fffffffe6a0) at git.c:509\n> > > \n> > > Bisection reveals the following culprit:\n> > > \n> > > commit 73cf0822b2a4ffa7ad559d1f0772e39718fc7776\n> > > Author: Julian Phillips \n> > > Date: Sun Oct 25 21:28:11 2009 +0000\n> > > \n> > > remote: Make ref_remove_duplicates faster for large numbers of refs\n> > \n> > Can't reproduce because I don't know how to create duplicate refs, but does\n> > the following help?\n\nNope.\n\n> > remote.c | 2 ++\n> > 1 files changed, 2 insertions(+), 0 deletions(-)\n> > \n> > diff --git a/remote.c b/remote.c\n> > index 4f9f0cc..10cc985 100644\n> > --- a/remote.c\n> > +++ b/remote.c\n> > @@ -754,6 +754,8 @@ void ref_remove_duplicates(struct ref *ref_map)\n> > \t\t\tprev->next = ref_map->next;\n> > \t\t\tfree(ref_map->peer_ref);\n> > \t\t\tfree(ref_map);\n> > +\t\t\tref_map = next;\n> \n> You don't need this line (this is taken care of in the for(...)).\n> \n> > +\t\t\tcontinue;\n> \n> Ack. This one however, you do need. Good catch.\n\nWithout the \"ref_map = next\" there is no change: glibc still complains \nabout corruption and abort the execution. With the \"ref_map = next\" \nthen git simply segfaults.\n\nI simply have zero time to investigate the issue myself now \nunfortunately.\n\n\nNicolas\n"},{"id":"127501","messageId":"alpine.LNX.2.00.0911130910150.17726@reaper.quantumfyre.co.uk","threadId":"21596","inReplyTo":"alpine.LFD.2.00.0911122345450.16711@xanadu.home","subject":"Re: Git in next is broken","fromName":"Julian Phillips","fromEmail":"julian@quantumfyre.co.uk","sentAt":"2009-11-13T09:14:28Z","receivedAt":"2009-11-13T09:14:28Z","isPatch":false,"sender":{"key":"julian@quantumfyre.co.uk","avatar":"https://avatars.githubusercontent.com/u/948888?v=4"},"body":"On Thu, 12 Nov 2009, Nicolas Pitre wrote:\n\n> On Thu, 12 Nov 2009, Julian Phillips wrote:\n>\n>> On Thu, 12 Nov 2009, Ren? Scharfe wrote:\n>>\n>>> Nicolas Pitre schrieb:\n>>>> Simply issuing a \"git fetch\" in my copy of git.git makes glibc complain\n>>>> with this:\n>>>>\n>>>> *** glibc detected *** git: corrupted double-linked list:\n>>>> 0x0000000000974180 ***\n\n>>> Can't reproduce because I don't know how to create duplicate refs, but does\n>>> the following help?\n>\n> Nope.\n>\n>>> remote.c | 2 ++\n>>> 1 files changed, 2 insertions(+), 0 deletions(-)\n>>>\n>>> diff --git a/remote.c b/remote.c\n>>> index 4f9f0cc..10cc985 100644\n>>> --- a/remote.c\n>>> +++ b/remote.c\n>>> @@ -754,6 +754,8 @@ void ref_remove_duplicates(struct ref *ref_map)\n>>> \t\t\tprev->next = ref_map->next;\n>>> \t\t\tfree(ref_map->peer_ref);\n>>> \t\t\tfree(ref_map);\n>>> +\t\t\tref_map = next;\n>>\n>> You don't need this line (this is taken care of in the for(...)).\n>>\n>>> +\t\t\tcontinue;\n>>\n>> Ack. This one however, you do need. Good catch.\n>\n> Without the \"ref_map = next\" there is no change: glibc still complains\n> about corruption and abort the execution. With the \"ref_map = next\"\n> then git simply segfaults.\n>\n> I simply have zero time to investigate the issue myself now\n> unfortunately.\n\nI was half right about \"ref_map = next\", I had forgotten about setting \nprev in the for(...). For me, the following fixes it on MacOS (I don't \nhave time to test on Linux right now):\n\ndiff --git a/remote.c b/remote.c\nindex 4f9f0cc..6195a58 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -754,6 +754,8 @@ void ref_remove_duplicates(struct ref *ref_map)\n prev->next = ref_map->next;\n free(ref_map->peer_ref);\n free(ref_map);\n+ ref_map = prev; // Keep the same prev.\n+ continue;\n }\n\n item = string_list_insert(ref_map->peer_ref->name, &refs);\n\n-- \nJulian\n\n ---\nC'est magnifique, mais ce n'est pas l'Informatique.\n \t\t-- Bosquet [on seeing the IBM 4341]\n"},{"id":"127512","messageId":"alpine.LFD.2.00.0911131152120.16711@xanadu.home","threadId":"21596","inReplyTo":"alpine.LNX.2.00.0911130910150.17726@reaper.quantumfyre.co.uk","subject":"Re: Git in next is broken","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2009-11-13T16:54:30Z","receivedAt":"2009-11-13T16:54:30Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Fri, 13 Nov 2009, Julian Phillips wrote:\n\n> On Thu, 12 Nov 2009, Nicolas Pitre wrote:\n> \n> > Without the \"ref_map = next\" there is no change: glibc still complains\n> > about corruption and abort the execution. With the \"ref_map = next\"\n> > then git simply segfaults.\n> \n> I was half right about \"ref_map = next\", I had forgotten about setting prev in\n> the for(...). For me, the following fixes it on MacOS (I don't have time to\n> test on Linux right now):\n> \n> diff --git a/remote.c b/remote.c\n> index 4f9f0cc..6195a58 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -754,6 +754,8 @@ void ref_remove_duplicates(struct ref *ref_map)\n> prev->next = ref_map->next;\n> free(ref_map->peer_ref);\n> free(ref_map);\n> + ref_map = prev; // Keep the same prev.\n> + continue;\n> }\n> \n> item = string_list_insert(ref_map->peer_ref->name, &refs);\n\nYep, this fixes it for me on Linux too. Please resend to Junio with my \nACK.\n\n\nNicolas\n"},{"id":"127516","messageId":"loom.20091113T211800-4@post.gmane.org","threadId":"21596","inReplyTo":"alpine.LFD.2.00.0911131152120.16711@xanadu.home","subject":"Re: Git in next is broken","fromName":"Eric Raible","fromEmail":"raible@gmail.com","sentAt":"2009-11-13T20:20:47Z","receivedAt":"2009-11-13T20:20:47Z","isPatch":false,"sender":{"key":"raible@gmail.com","avatar":null},"body":"Nicolas Pitre fluxnic.net> writes:\n\n > diff --git a/remote.c b/remote.c\n> > index 4f9f0cc..6195a58 100644\n> > --- a/remote.c\n> > +++ b/remote.c\n> > @@ -754,6 +754,8 @@ void ref_remove_duplicates(struct ref *ref_map)\n> > prev->next = ref_map->next;\n> > free(ref_map->peer_ref);\n> > free(ref_map);\n> > + ref_map = prev; // Keep the same prev.\n> > + continue;\n> > }\n> > \n\nThis is one of those example where the comment is essentially useless.\nWouldn't something like \"// Skip the freed item\" be more useful?\n\n- Eric\n"},{"id":"127519","messageId":"20091113212557.40912.20173.julian@quantumfyre.co.uk","threadId":"21596","inReplyTo":"alpine.LFD.2.00.0911131152120.16711@xanadu.home","subject":"[PATCH] remote: Fix glibc error in ref_remove_duplicates","fromName":"Julian Phillips","fromEmail":"julian@quantumfyre.co.uk","sentAt":"2009-11-13T21:25:56Z","receivedAt":"2009-11-13T21:25:56Z","isPatch":true,"sender":{"key":"julian@quantumfyre.co.uk","avatar":"https://avatars.githubusercontent.com/u/948888?v=4"},"body":"In ref_remove_duplicates, when we encounter a duplicate and remove it\nfrom the list we need to make sure that the prev pointer stays\npointing at the last entry and also skip over adding the just freed\nentry to the string_list.\n\nPreviously fetch could crash with:\n*** glibc detected *** git: corrupted double-linked list: ...\n\nAlso add a test to try and catch problems with duplicate removal in\nthe future.\n\nAcked-by: Nicolas Pitre \nSigned-off-by: Julian Phillips \n---\n\nThanks to Rene for pointing me at the problem before I even looked at\nit. Made it much easier to figure out what was going wrong. :)\n\n remote.c | 2 ++\n t/t5510-fetch.sh | 11 +++++++++++\n 2 files changed, 13 insertions(+), 0 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 4f9f0cc..002f790 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -754,6 +754,8 @@ void ref_remove_duplicates(struct ref *ref_map)\n \t\t\tprev->next = ref_map->next;\n \t\t\tfree(ref_map->peer_ref);\n \t\t\tfree(ref_map);\n+\t\t\tref_map = prev; // don't change the prev pointer.\n+\t\t\tcontinue;\n \t\t}\n \n \t\titem = string_list_insert(ref_map->peer_ref->name, &refs);\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex d13c806..169af1e 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -341,4 +341,15 @@ test_expect_success 'fetch into the current branch with --update-head-ok' '\n \n '\n \n+test_expect_success \"should be able to fetch with duplicate refspecs\" '\n+ mkdir dups &&\n+ cd dups &&\n+ git init &&\n+ git config branch.master.remote three &&\n+ git config remote.three.url ../three/.git &&\n+ git config remote.three.fetch +refs/heads/*:refs/remotes/origin/* &&\n+ git config --add remote.three.fetch +refs/heads/*:refs/remotes/origin/* &&\n+ git fetch three\n+'\n+\n test_done\n-- \n1.6.5.rc2\n"}]}