Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.85k stars 523 forks source link

cannot weaken refs to read only values #6930

Closed p5pRT closed 20 years ago

p5pRT commented 20 years ago

Migrated from rt.perl.org#24506 (status was 'resolved')

Searchable as RT24506$

p5pRT commented 20 years ago

From fergal@esatclear.ie

with 5.8.2 and 5.6.1 (and presumably 5.6.2)

perl -MScalar​::Util=weaken -e '$a=\"h";weaken($a)' Modification of a read-only value attempted at -e line 1

this means that you probaly should be doing

eval {   local $SIG{__DIE__};   weaken($a); };

wherever you were using it before. Patch attached to fix this by adding PERL_MAGIC_backref to the list of things that can be done to a readonly value.

I added some tests\, everything still passes\, is there some other reason not to allow this?

F

p5pRT commented 20 years ago

From fergal@esatclear.ie

weaken.patch ```diff --- ./ext/List/Util/t/weak.t.orig 2003-11-16 20:05:28.000000000 +0000 +++ ./ext/List/Util/t/weak.t 2003-11-16 21:04:47.000000000 +0000 @@ -35,7 +35,7 @@ eval <<'EOT' unless $skip; use Scalar::Util qw(weaken isweak); -print "1..17\n"; +print "1..20\n"; ######################### End of black magic. @@ -44,6 +44,7 @@ sub ok { ++$cnt; if($_[0]) { print "ok $cnt\n"; } else {print "not ok $cnt\n"; } + return $_[0]; } $| = 1; @@ -205,6 +206,17 @@ ok(isweak($x->{Y})); ok(!isweak($x->{Z})); +# +# Case 7: test weaken on a read only ref +# + +$a = \"hello"; +$b = $a; +eval{weaken($b)}; +# we didn't die +ok($@ eq "") or print "#died with $@\n"; +ok(isweak($b)); +ok($$b eq "hello") or print "#b is '$$b'\n"; package Dest; --- ./lib/Exporter.pm.orig 2003-11-09 23:03:20.000000000 +0000 +++ ./lib/Exporter.pm 2003-11-16 20:53:12.000000000 +0000 @@ -30,6 +30,12 @@ my $pkg = shift; my $callpkg = caller($ExportLevel); + if (@_ and $pkg eq "Exporter" and $_[0] eq "import") + { + *{$callpkg."::import"} = \&import; + return; + } + # We *need* to treat @{"$pkg\::EXPORT_FAIL"} since Carp uses it :-( my($exports, $fail) = (\@{"$pkg\::EXPORT"}, \@{"$pkg\::EXPORT_FAIL"}); return export $pkg, $callpkg, @_ @@ -103,6 +109,12 @@ @ISA = qw(Exporter); @EXPORT_OK = qw(munge frobnicate); # symbols to export on request +or + + package YourModule; + use Exporter 'import'; # gives you Exporter's import() method directly + @EXPORT_OK = qw(munge frobnicate); # symbols to export on request + In other files which wish to use YourModule: use ModuleName qw(frobnicate); # import listed symbols @@ -290,6 +302,19 @@ - or people using your package will get very unexplained results! +=head2 Exporting without inheriting from Exporter + +By including Exporter in your @ISA you inherit an Exporter's import() method +but you also inherit several other helper methods which you probably don't +want. To avoid this you can do + + package YourModule; + use Exporter qw( import ); + +which will export Exporter's own import() method into YourModule. +Everything will work as before but you won't need to include Exporter in +@YourModule::ISA. + =head2 Module Version Checking The Exporter module will convert an attempt to import a number from a --- ./lib/Exporter.t.orig 2003-11-09 23:03:30.000000000 +0000 +++ ./lib/Exporter.t 2003-11-16 21:13:15.000000000 +0000 @@ -21,7 +21,7 @@ } -print "1..26\n"; +print "1..28\n"; require Exporter; ok( 1, 'Exporter compiled' ); @@ -196,3 +196,21 @@ Moving::Target->import (bar); ::ok (bar eq "bar", "imported bar after EXPORT_OK changed"); + +package The::Import; + +use Exporter 'import'; + +eval { import() }; +::ok(\&import == \&Exporter::import, "imported the import routine"); + +@EXPORT = qw( wibble ); +sub wibble {return "wobble"}; + +package Use::The::Import; + +The::Import->import; + +my $val = eval { wibble() }; +::ok($val eq "wobble", "exported importer worked"); + --- ./sv.c.orig 2003-11-16 20:01:12.000000000 +0000 +++ ./sv.c 2003-11-16 20:14:25.000000000 +0000 @@ -4649,6 +4649,7 @@ && how != PERL_MAGIC_bm && how != PERL_MAGIC_fm && how != PERL_MAGIC_sv + && how != PERL_MAGIC_backref ) { Perl_croak(aTHX_ PL_no_modify); ```
p5pRT commented 20 years ago

From fergal@esatclear.ie

Previous patch included an unrelated patch to Exporter.pm\, this one is for weaken only\,

F

p5pRT commented 20 years ago

From fergal@esatclear.ie

weaken.patch ```diff +++ ./ext/List/Util/t/weak.t 2003-11-16 21:04:47.000000000 +0000 @@ -35,7 +35,7 @@ eval <<'EOT' unless $skip; use Scalar::Util qw(weaken isweak); -print "1..17\n"; +print "1..20\n"; ######################### End of black magic. @@ -44,6 +44,7 @@ sub ok { ++$cnt; if($_[0]) { print "ok $cnt\n"; } else {print "not ok $cnt\n"; } + return $_[0]; } $| = 1; @@ -205,6 +206,17 @@ ok(isweak($x->{Y})); ok(!isweak($x->{Z})); +# +# Case 7: test weaken on a read only ref +# + +$a = \"hello"; +$b = $a; +eval{weaken($b)}; +# we didn't die +ok($@ eq "") or print "#died with $@\n"; +ok(isweak($b)); +ok($$b eq "hello") or print "#b is '$$b'\n"; package Dest; --- ./sv.c.orig 2003-11-16 20:01:12.000000000 +0000 +++ ./sv.c 2003-11-16 20:14:25.000000000 +0000 @@ -4649,6 +4649,7 @@ && how != PERL_MAGIC_bm && how != PERL_MAGIC_fm && how != PERL_MAGIC_sv + && how != PERL_MAGIC_backref ) { Perl_croak(aTHX_ PL_no_modify); ```
p5pRT commented 20 years ago

From fergal@esatclear.ie

3rd attempt to get this mail through to the list...

With blead\, 5.8.2 and probably all perls that have weak refs

perl -MScalar​::Util=weaken -e '$a=\"h";weaken($a)' Modification of a read-only value attempted at -e line 1

Any good reason for this? It's not documented and it's a pain.

If you want to weaken refs of unknown origin (eg in a generic cache module) then you actually have to do

eval {   local $SIG{__DIE__};   weaken($ref); };

And also you end up with read only refs that don't get garbage collected when they should [1].

Patch for blead attached to fix this by adding PERL_MAGIC_backref to the list of things that can be done to a readonly value. Also applied to 5.8 and maybe 5.6.

I added some tests\, everything still passes\, is there some other reason not to allow this?

F

[1] It's rare that you get a GC'able read only ref but

$r = eval '\"hello"'

produces one.

p5pRT commented 20 years ago

From fergal@esatclear.ie

weaken.patch ```diff --- ./ext/List/Util/t/weak.t.orig 2003-12-02 23:13:36.000000000 +0000 +++ ./ext/List/Util/t/weak.t 2003-12-02 23:13:47.000000000 +0000 @@ -35,7 +35,7 @@ eval <<'EOT' unless $skip; use Scalar::Util qw(weaken isweak); -print "1..17\n"; +print "1..22\n"; ######################### End of black magic. @@ -44,6 +44,7 @@ sub ok { ++$cnt; if($_[0]) { print "ok $cnt\n"; } else {print "not ok $cnt\n"; } + return $_[0]; } $| = 1; @@ -205,6 +206,20 @@ ok(isweak($x->{Y})); ok(!isweak($x->{Z})); +# +# Case 7: test weaken on a read only ref +# + +$a = eval '\"hello"'; +ok(ref($a)) or print "# didn't get a ref from eval\n"; +$b = $a; +eval{weaken($b)}; +# we didn't die +ok($@ eq "") or print "# died with $@\n"; +ok(isweak($b)); +ok($$b eq "hello") or print "# b is '$$b'\n"; +$a=""; +ok(not $b) or print "# b didn't go away\n"; package Dest; --- ./sv.c.orig 2003-12-02 23:13:37.000000000 +0000 +++ ./sv.c 2003-12-02 23:13:47.000000000 +0000 @@ -4896,6 +4896,7 @@ && how != PERL_MAGIC_bm && how != PERL_MAGIC_fm && how != PERL_MAGIC_sv + && how != PERL_MAGIC_backref ) { Perl_croak(aTHX_ PL_no_modify); ```
p5pRT commented 20 years ago

From @rgs

Fergal Daly wrote​:

With blead\, 5.8.2 and probably all perls that have weak refs

perl -MScalar​::Util=weaken -e '$a=\"h";weaken($a)' Modification of a read-only value attempted at -e line 1

Any good reason for this? It's not documented and it's a pain.

If you want to weaken refs of unknown origin (eg in a generic cache module) then you actually have to do

eval { local $SIG{__DIE__}; weaken($ref); };

And also you end up with read only refs that don't get garbage collected when they should [1].

Yes.

Patch for blead attached to fix this by adding PERL_MAGIC_backref to the list of things that can be done to a readonly value. Also applied to 5.8 and maybe 5.6.

This makes me a bit nervous\, but I think it's correct ; in fact I was looking at it currently.

I added some tests\, everything still passes\, is there some other reason not to allow this?

I noticed a Point of Style : you patched the core but added a test in a CPAN module. This test will obviously fail when backported to CPAN. There are two solutions -- either put the test in a separate file\, or skip it for $] \< 5.009 or $] == 5.009 && ! $ENV{PERL_CORE}.

Graham\, any comments on this ?

p5pRT commented 20 years ago

From @gbarr

On 2 Dec 2003\, at 23​:31\, Rafael Garcia-Suarez wrote​:

I added some tests\, everything still passes\, is there some other reason not to allow this?

I noticed a Point of Style : you patched the core but added a test in a CPAN module. This test will obviously fail when backported to CPAN. There are two solutions -- either put the test in a separate file\, or skip it for $] \< 5.009 or $] == 5.009 && ! $ENV{PERL_CORE}.

Graham\, any comments on this ?

I would make it so that the test is skipped for $] \< 5.009

Graham.

p5pRT commented 20 years ago

From fergal@esatclear.ie

On Wednesday 03 December 2003 22​:19\, Graham Barr via RT wrote​:

I would make it so that the test is skipped for $] \< 5.009

Graham.

Before I do that\, any chance I could convert the script to use Test​::More as the testing framework? It makes everything a bit easier. I'll even convert the other test scripts for free\,

F

p5pRT commented 20 years ago

From @rgs

Graham Barr wrote​:

On 2 Dec 2003\, at 23​:31\, Rafael Garcia-Suarez wrote​:

I added some tests\, everything still passes\, is there some other reason not to allow this?

I noticed a Point of Style : you patched the core but added a test in a CPAN module. This test will obviously fail when backported to CPAN. There are two solutions -- either put the test in a separate file\, or skip it for $] \< 5.009 or $] == 5.009 && ! $ENV{PERL_CORE}.

Graham\, any comments on this ?

I would make it so that the test is skipped for $] \< 5.009

OK. I thus applied Fergal's patch (thanks) as​:

Change 21955 by rgs@​rgs-home on 2003/12/25 18​:59​:54

  Subject​: [perl #24506] [PATCH] cannot weaken refs to read only values   From​: Fergal Daly \fergal@&#8203;esatclear\.ie   Date​: Tue\, 2 Dec 2003 23​:18​:18 +0000   Message-Id​: \200312022318\.18353\.fergal@&#8203;esatclear\.ie  
  (tweaked so the test is skipped on perls \< 5.9.0)

Affected files ...

... //depot/perl/ext/List/Util/t/weak.t#3 edit ... //depot/perl/sv.c#707 edit

p5pRT commented 20 years ago

@rgs - Status changed from 'new' to 'resolved'

p5pRT commented 20 years ago

From @gbarr

On 25 Dec 2003\, at 19​:25\, Rafael Garcia-Suarez wrote​:

Graham Barr wrote​:

On 2 Dec 2003\, at 23​:31\, Rafael Garcia-Suarez wrote​:

I added some tests\, everything still passes\, is there some other reason not to allow this?

I noticed a Point of Style : you patched the core but added a test in a CPAN module. This test will obviously fail when backported to CPAN. There are two solutions -- either put the test in a separate file\, or skip it for $] \< 5.009 or $] == 5.009 && ! $ENV{PERL_CORE}.

Graham\, any comments on this ?

I would make it so that the test is skipped for $] \< 5.009

OK. I thus applied Fergal's patch (thanks) as​:

Change 21955 by rgs@​rgs-home on 2003/12/25 18​:59​:54

    Subject&#8203;: \[perl \#24506\] \[PATCH\] cannot weaken refs to read only 

values From​: Fergal Daly \fergal@&#8203;esatclear\.ie Date​: Tue\, 2 Dec 2003 23​:18​:18 +0000 Message-Id​: \200312022318\.18353\.fergal@&#8203;esatclear\.ie

    \(tweaked so the test is skipped on perls \< 5\.9\.0\)

I saw that\, Thanks.

I am a bit tied up with family stuff over the holidays\, but I will do a CPAN release when I can.

Graham.