{"thread":{"id":"24429","subject":"[PATCH] git-svn: write memoized data explicitly to avoid Storable bug","startedAt":"2010-07-18T12:17:49Z","lastAt":"2010-07-20T19:02:23Z","messageCount":5,"participants":["Sergey Vlasov","Eric Wong","Junio C Hamano","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"145746","messageId":"1279455469-6384-1-git-send-email-vsu@altlinux.ru","threadId":"24429","inReplyTo":null,"subject":"[PATCH] git-svn: write memoized data explicitly to avoid Storable bug","fromName":"Sergey Vlasov","fromEmail":"vsu@altlinux.ru","sentAt":"2010-07-18T12:17:49Z","receivedAt":"2010-07-18T12:17:49Z","isPatch":true,"sender":{"key":"vsu@altlinux.ru","avatar":"https://avatars.githubusercontent.com/u/616082?v=4"},"body":"Apparently using the Storable module during global destruction is\nunsafe - there is a bug which can cause segmentation faults:\n\n http://rt.cpan.org/Public/Bug/Display.html?id=36087\n http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=482355\n\nThe persistent memoization support introduced in commit 8bff7c538\nrelied on global destruction to write cached data, which was leading\nto segfaults in some Perl configurations. Calling Memoize::unmemoize\nin the END block forces the cache writeout to be performed earlier,\nthus avoiding the bug.\n\nSigned-off-by: Sergey Vlasov \n---\n git-svn.perl | 16 ++++++++++++++++\n 1 files changed, 16 insertions(+), 0 deletions(-)\n\n In my case segfaults happen only when the perl-IO-Compress module is\n installed - apparently the attempt to load Compress::Zlib (now\n provided by IO::Compress) changes something.\n\n This unaswered report is also suspicious:\n\n http://thread.gmane.org/gmane.comp.version-control.git/142161\n\n (when running ./t9151-svn-mergeinfo.sh -v, the segfault should be\n visible: \"error: git-svn died of signal 11\").\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 19d6848..c416358 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -3169,6 +3169,22 @@ sub has_no_changes {\n \t\t\tLIST_CACHE => 'FAULT',\n \t\t;\n \t}\n+\n+\tsub unmemoize_svn_mergeinfo_functions {\n+\t\treturn if not $memoized;\n+\t\t$memoized = 0;\n+\n+\t\tMemoize::unmemoize 'lookup_svn_merge';\n+\t\tMemoize::unmemoize 'check_cherry_pick';\n+\t\tMemoize::unmemoize 'has_no_changes';\n+\t}\n+}\n+\n+END {\n+\t# Force cache writeout explicitly instead of waiting for\n+\t# global destruction to avoid segfault in Storable:\n+\t# http://rt.cpan.org/Public/Bug/Display.html?id=36087\n+\tunmemoize_svn_mergeinfo_functions();\n }\n \n sub parents_exclude {\n-- \n1.6.0.2.321.g8406\n"},{"id":"145784","messageId":"20100719063903.GA3680@dcvr.yhbt.net","threadId":"24429","inReplyTo":"1279455469-6384-1-git-send-email-vsu@altlinux.ru","subject":"Re: [PATCH] git-svn: write memoized data explicitly to avoid Storable bug","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2010-07-19T06:39:03Z","receivedAt":"2010-07-19T06:39:03Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Sergey Vlasov wrote:\n> Apparently using the Storable module during global destruction is\n> unsafe - there is a bug which can cause segmentation faults:\n> \n> http://rt.cpan.org/Public/Bug/Display.html?id=36087\n> http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=482355\n> \n> The persistent memoization support introduced in commit 8bff7c538\n> relied on global destruction to write cached data, which was leading\n> to segfaults in some Perl configurations. Calling Memoize::unmemoize\n> in the END block forces the cache writeout to be performed earlier,\n> thus avoiding the bug.\n> \n> Signed-off-by: Sergey Vlasov \n\nThanks Sergey,\n\nAcked-by: Eric Wong \n\n...and pushed to git://git.bogomips.org/git-svn\n\n-- \nEric Wong\n"},{"id":"145798","messageId":"7vlj97pgv7.fsf@alter.siamese.dyndns.org","threadId":"24429","inReplyTo":"20100719063903.GA3680@dcvr.yhbt.net","subject":"Re: [PATCH] git-svn: write memoized data explicitly to avoid Storable bug","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-07-19T16:37:48Z","receivedAt":"2010-07-19T16:37:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n> Thanks Sergey,\n>\n> Acked-by: Eric Wong \n>\n> ...and pushed to git://git.bogomips.org/git-svn\n\nThanks all; pulled.\n"},{"id":"145872","messageId":"AANLkTime7QQZGLXmXg_X3W7CsbyLe5NbPKcqs9dp0oaa@mail.gmail.com","threadId":"24429","inReplyTo":"1279455469-6384-1-git-send-email-vsu@altlinux.ru","subject":"Re: [PATCH] git-svn: write memoized data explicitly to avoid Storable bug","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-07-20T17:32:31Z","receivedAt":"2010-07-20T17:32:31Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Sun, Jul 18, 2010 at 12:17, Sergey Vlasov wrote:\n> Apparently using the Storable module during global destruction is\n> unsafe - there is a bug which can cause segmentation faults:\n>\n>  http://rt.cpan.org/Public/Bug/Display.html?id=36087\n>  http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=482355\n\nI did some investigation into the upstream issue:\nhttps://rt.cpan.org/Ticket/Display.html?id=36087#txn-806832\n\n> The persistent memoization support introduced in commit 8bff7c538\n> relied on global destruction to write cached data, which was leading\n> to segfaults in some Perl configurations.  Calling Memoize::unmemoize\n> in the END block forces the cache writeout to be performed earlier,\n> thus avoiding the bug.\n\nMaybe I'm missing something obvious, but this seems like the wrong\nsolution. The core issue is that we don't want to clean up during\nglobal destruction, but then we should just do:\n\n sub DESTROY {\n return if not $memoized;\n $memoized = 0;\n\n Memoize::unmemoize 'lookup_svn_merge';\n Memoize::unmemoize 'check_cherry_pick';\n Memoize::unmemoize 'has_no_changes';\n }\n\nThat should work since memoize_svn_mergeinfo_functions(); is being\ncalled in find_extra_svn_parents, which is a Git::SVN object\nmethod. Can you try this and confirm/deny? I can't because I can't get\nthe original to segfault on my box when run within git-svn.\n"},{"id":"145888","messageId":"20100720190223.GB2732@dcvr.yhbt.net","threadId":"24429","inReplyTo":"AANLkTime7QQZGLXmXg_X3W7CsbyLe5NbPKcqs9dp0oaa@mail.gmail.com","subject":"Re: [PATCH] git-svn: write memoized data explicitly to avoid Storable bug","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2010-07-20T19:02:23Z","receivedAt":"2010-07-20T19:02:23Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Ævar Arnfjörð Bjarmason wrote:\n> On Sun, Jul 18, 2010 at 12:17, Sergey Vlasov wrote:\n> > Apparently using the Storable module during global destruction is\n> > unsafe - there is a bug which can cause segmentation faults:\n> >\n> >  http://rt.cpan.org/Public/Bug/Display.html?id=36087\n> >  http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=482355\n> \n> I did some investigation into the upstream issue:\n> https://rt.cpan.org/Ticket/Display.html?id=36087#txn-806832\n> \n> > The persistent memoization support introduced in commit 8bff7c538\n> > relied on global destruction to write cached data, which was leading\n> > to segfaults in some Perl configurations.  Calling Memoize::unmemoize\n> > in the END block forces the cache writeout to be performed earlier,\n> > thus avoiding the bug.\n> \n> Maybe I'm missing something obvious, but this seems like the wrong\n> solution. The core issue is that we don't want to clean up during\n> global destruction, but then we should just do:\n> \n> sub DESTROY {\n> return if not $memoized;\n> $memoized = 0;\n> \n> Memoize::unmemoize 'lookup_svn_merge';\n> Memoize::unmemoize 'check_cherry_pick';\n> Memoize::unmemoize 'has_no_changes';\n> }\n\nI haven't looked at this issue in-depth, but I believe the problem is\ntriggered due to Memoize::Storable trying to use Storable::nstore\nin its own DESTROY function. So trying to do the same in our own\nDESTROY would be just as bad.\n\n> That should work since memoize_svn_mergeinfo_functions(); is being\n> called in find_extra_svn_parents, which is a Git::SVN object\n> method. Can you try this and confirm/deny? I can't because I can't get\n> the original to segfault on my box when run within git-svn.\n\nI wasn't able to reproduce the segfault on my systems, either, but\nit seems plausible it would only happen on some systems.\n\n-- \nEric Wong\n"}]}