[PATCH] Deep recursion warning with %Module::CoreList::version #15292
Comments
From @ntyniIterating over %Module::CoreList::version elements for Perl >= 5.23.2 % perl -MModule::CoreList -wE 'say scalar keys %{$Module::CoreList::version{5.023002}}' Proposed patch attached, assuming "no warnings 'recursion'" is fine Perl Info |
From @ntyni0001-Squelch-deep-recursion-warnings-with-Module-CoreList.patchFrom 2575bfb69473b9831d4a3fbb2fdf5279ece0ab70 Mon Sep 17 00:00:00 2001
From: Niko Tyni <ntyni@debian.org>
Date: Sun, 24 Apr 2016 17:27:58 +0300
Subject: [PATCH] Squelch deep recursion warnings with
%Module::CoreList::version
The version hashes form a chain that gets longer with each Perl
version. Since 5.23.2, the length of the chain has exceeded 100 entries,
causing deep recursion warnings when inflating later hashes.
% perl -MModule::CoreList -wE 'say scalar keys %{$Module::CoreList::version{5.023002}}'
Deep recursion on subroutine "Module::CoreList::TieHashDelta::FIRSTKEY" at /usr/share/perl/5.22/Module/CoreList/TieHashDelta.pm line 54.
622
The recursion is deep but not infinite, so silence the warning.
---
dist/Module-CoreList/lib/Module/CoreList/TieHashDelta.pm | 3 +++
dist/Module-CoreList/t/corelist.t | 8 +++++++-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/dist/Module-CoreList/lib/Module/CoreList/TieHashDelta.pm b/dist/Module-CoreList/lib/Module/CoreList/TieHashDelta.pm
index 94507d7..c58bc4c 100644
--- a/dist/Module-CoreList/lib/Module/CoreList/TieHashDelta.pm
+++ b/dist/Module-CoreList/lib/Module/CoreList/TieHashDelta.pm
@@ -48,6 +48,9 @@ sub FIRSTKEY {
my ($self) = @_;
if (not $self->{keys_inflated}) {
+ # exceeds the warning limit of 100 calls since 5.23.2
+ no warnings 'recursion';
+
# This inflates the whole set of hashes... Somewhat expensive, but saves
# many tied hash calls later.
my @parent_keys;
diff --git a/dist/Module-CoreList/t/corelist.t b/dist/Module-CoreList/t/corelist.t
index db09f48..4adbbe9 100644
--- a/dist/Module-CoreList/t/corelist.t
+++ b/dist/Module-CoreList/t/corelist.t
@@ -1,7 +1,7 @@
#!perl -w
use strict;
use Module::CoreList;
-use Test::More tests => 32;
+use Test::More tests => 34;
BEGIN { require_ok('Module::CoreList'); }
@@ -105,6 +105,12 @@ is(Module::CoreList::removed_from('CPANPLUS::inc'), 5.010001,
cmp_ok($warnings_count, '==', 0,
"an undefined version does not produce warnings rt#123556");
+
+ ok(keys %{$Module::CoreList::version{5.023002}} > 0,
+ "have recent enough data to test for deep recursion warnings");
+
+ cmp_ok($warnings_count, '==', 0,
+ "no deep recursion warnings for newer perl versions");
}
ok(! defined(Module::CoreList->find_version()),
--
2.8.0.rc3
|
From @cpansproutOn Sun Apr 24 12:35:03 2016, ntyni@debian.org wrote:
I have used it in 5.8, so it should be fine. I think this should be fixed before we release 5.24. Your patch looks OK to me, but I have not tested it. -- Father Chrysostomos |
|
The RT System itself - Status changed from 'new' to 'open' |
From @iabynOn Sun, Apr 24, 2016 at 02:48:46PM -0700, Father Chrysostomos via RT wrote:
+1 Its due to a tied lookup recursively looking up parent perl versions In the tests the warning only appears occasionally during smoking due to Post-5.24 we really ought to fix it properly (iterate rather than -- |
From @rjbs* Niko Tyni <perlbug-followup@perl.org> [2016-04-24T15:35:04]
Thanks, applied as bb8df43. -- |
From @ap* Dave Mitchell <davem@iabyn.com> [2016-04-25 12:23]:
I’m working on that. |
From @bulk88On Mon Apr 25 00:29:23 2016, davem wrote:
For the record I already encountered C stack recursion exhaustion bugs before with TieHashDelta https://rt-archive.perl.org/perl5/Ticket/Display.html?id=121974 and fixed in http://perl5.git.perl.org/perl.git/commit/42484eedeb4abcee1d893010daa67c1a42ec6424 . More recursion instead of looping/tailcalling bugs dont surprise me. -- |
From @iabynOn Wed, Apr 27, 2016 at 01:05:43PM +0200, Aristotle Pagaltzis wrote:
Any progress? -- |
From @apWork on the CoreList replacement has been slow, but I have now pushed |
From @jkeenanOn Wed, 22 Jun 2016 21:19:09 GMT, aristotle wrote:
This looks promising. Have you been able to make further progress? Thank you very much. -- |
From @ap* James E Keenan via RT <perlbug-followup@perl.org> [2016-12-27 02:24]:
Yeah, applied as 8bbcf42 some time ago. Sorry I forgot the ticket. |
From @jkeenanOn Tue, 27 Dec 2016 04:54:47 GMT, aristotle wrote:
Niko, have we addressed the problem about which you originally posted in this RT? If so, then we can close the ticket. Thank you very much. -- |
Migrated from rt.perl.org#127981 (status was 'open')
Searchable as RT127981$
The text was updated successfully, but these errors were encountered: