# [PATCH] git-svn: write memoized data explicitly to avoid Storable bug 5 messages from 2010-07-18 to 2010-07-20. Participants: Sergey Vlasov, Eric Wong, Junio C Hamano, Ævar Arnfjörð Bjarmason. Thread: https://gitlist.dev/t/24429 ## Sergey Vlasov, 2010-07-18 12:17 Subject: [PATCH] git-svn: write memoized data explicitly to avoid Storable bug Message-ID: <1279455469-6384-1-git-send-email-vsu@altlinux.ru> URL: https://gitlist.dev/e/1279455469-6384-1-git-send-email-vsu%40altlinux.ru ``` Apparently using the Storable module during global destruction is unsafe - there is a bug which can cause segmentation faults: http://rt.cpan.org/Public/Bug/Display.html?id=36087 http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=482355 The persistent memoization support introduced in commit 8bff7c538 relied on global destruction to write cached data, which was leading to segfaults in some Perl configurations. Calling Memoize::unmemoize in the END block forces the cache writeout to be performed earlier, thus avoiding the bug. Signed-off-by: Sergey Vlasov --- git-svn.perl | 16 ++++++++++++++++ 1 files changed, 16 insertions(+), 0 deletions(-) In my case segfaults happen only when the perl-IO-Compress module is installed - apparently the attempt to load Compress::Zlib (now provided by IO::Compress) changes something. This unaswered report is also suspicious: http://thread.gmane.org/gmane.comp.version-control.git/142161 (when running ./t9151-svn-mergeinfo.sh -v, the segfault should be visible: "error: git-svn died of signal 11"). diff --git a/git-svn.perl b/git-svn.perl index 19d6848..c416358 100755 --- a/git-svn.perl +++ b/git-svn.perl @@ -3169,6 +3169,22 @@ sub has_no_changes { LIST_CACHE => 'FAULT', ; } + + sub unmemoize_svn_mergeinfo_functions { + return if not $memoized; + $memoized = 0; + + Memoize::unmemoize 'lookup_svn_merge'; + Memoize::unmemoize 'check_cherry_pick'; + Memoize::unmemoize 'has_no_changes'; + } +} + +END { + # Force cache writeout explicitly instead of waiting for + # global destruction to avoid segfault in Storable: + # http://rt.cpan.org/Public/Bug/Display.html?id=36087 + unmemoize_svn_mergeinfo_functions(); } sub parents_exclude { -- 1.6.0.2.321.g8406 ``` ## Eric Wong, 2010-07-19 06:39 Subject: Re: [PATCH] git-svn: write memoized data explicitly to avoid Storable bug Message-ID: <20100719063903.GA3680@dcvr.yhbt.net> URL: https://gitlist.dev/e/20100719063903.GA3680%40dcvr.yhbt.net In-Reply-To: <1279455469-6384-1-git-send-email-vsu@altlinux.ru> ``` Sergey Vlasov wrote: > Apparently using the Storable module during global destruction is > unsafe - there is a bug which can cause segmentation faults: > > http://rt.cpan.org/Public/Bug/Display.html?id=36087 > http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=482355 > > The persistent memoization support introduced in commit 8bff7c538 > relied on global destruction to write cached data, which was leading > to segfaults in some Perl configurations. Calling Memoize::unmemoize > in the END block forces the cache writeout to be performed earlier, > thus avoiding the bug. > > Signed-off-by: Sergey Vlasov Thanks Sergey, Acked-by: Eric Wong ...and pushed to git://git.bogomips.org/git-svn -- Eric Wong ``` ## Junio C Hamano, 2010-07-19 16:37 Subject: Re: [PATCH] git-svn: write memoized data explicitly to avoid Storable bug Message-ID: <7vlj97pgv7.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vlj97pgv7.fsf%40alter.siamese.dyndns.org In-Reply-To: <20100719063903.GA3680@dcvr.yhbt.net> ``` Eric Wong writes: > Thanks Sergey, > > Acked-by: Eric Wong > > ...and pushed to git://git.bogomips.org/git-svn Thanks all; pulled. ``` ## Ævar Arnfjörð Bjarmason, 2010-07-20 17:32 Subject: Re: [PATCH] git-svn: write memoized data explicitly to avoid Storable bug Message-ID: URL: https://gitlist.dev/e/AANLkTime7QQZGLXmXg_X3W7CsbyLe5NbPKcqs9dp0oaa%40mail.gmail.com In-Reply-To: <1279455469-6384-1-git-send-email-vsu@altlinux.ru> ``` On Sun, Jul 18, 2010 at 12:17, Sergey Vlasov wrote: > Apparently using the Storable module during global destruction is > unsafe - there is a bug which can cause segmentation faults: > >  http://rt.cpan.org/Public/Bug/Display.html?id=36087 >  http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=482355 I did some investigation into the upstream issue: https://rt.cpan.org/Ticket/Display.html?id=36087#txn-806832 > The persistent memoization support introduced in commit 8bff7c538 > relied on global destruction to write cached data, which was leading > to segfaults in some Perl configurations.  Calling Memoize::unmemoize > in the END block forces the cache writeout to be performed earlier, > thus avoiding the bug. Maybe I'm missing something obvious, but this seems like the wrong solution. The core issue is that we don't want to clean up during global destruction, but then we should just do: sub DESTROY { return if not $memoized; $memoized = 0; Memoize::unmemoize 'lookup_svn_merge'; Memoize::unmemoize 'check_cherry_pick'; Memoize::unmemoize 'has_no_changes'; } That should work since memoize_svn_mergeinfo_functions(); is being called in find_extra_svn_parents, which is a Git::SVN object method. Can you try this and confirm/deny? I can't because I can't get the original to segfault on my box when run within git-svn. ``` ## Eric Wong, 2010-07-20 19:02 Subject: Re: [PATCH] git-svn: write memoized data explicitly to avoid Storable bug Message-ID: <20100720190223.GB2732@dcvr.yhbt.net> URL: https://gitlist.dev/e/20100720190223.GB2732%40dcvr.yhbt.net In-Reply-To: ``` Ævar Arnfjörð Bjarmason wrote: > On Sun, Jul 18, 2010 at 12:17, Sergey Vlasov wrote: > > Apparently using the Storable module during global destruction is > > unsafe - there is a bug which can cause segmentation faults: > > > >  http://rt.cpan.org/Public/Bug/Display.html?id=36087 > >  http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=482355 > > I did some investigation into the upstream issue: > https://rt.cpan.org/Ticket/Display.html?id=36087#txn-806832 > > > The persistent memoization support introduced in commit 8bff7c538 > > relied on global destruction to write cached data, which was leading > > to segfaults in some Perl configurations.  Calling Memoize::unmemoize > > in the END block forces the cache writeout to be performed earlier, > > thus avoiding the bug. > > Maybe I'm missing something obvious, but this seems like the wrong > solution. The core issue is that we don't want to clean up during > global destruction, but then we should just do: > > sub DESTROY { > return if not $memoized; > $memoized = 0; > > Memoize::unmemoize 'lookup_svn_merge'; > Memoize::unmemoize 'check_cherry_pick'; > Memoize::unmemoize 'has_no_changes'; > } I haven't looked at this issue in-depth, but I believe the problem is triggered due to Memoize::Storable trying to use Storable::nstore in its own DESTROY function. So trying to do the same in our own DESTROY would be just as bad. > That should work since memoize_svn_mergeinfo_functions(); is being > called in find_extra_svn_parents, which is a Git::SVN object > method. Can you try this and confirm/deny? I can't because I can't get > the original to segfault on my box when run within git-svn. I wasn't able to reproduce the segfault on my systems, either, but it seems plausible it would only happen on some systems. -- Eric Wong ```