From 27cc3b5a047e2e329f60a8c56fd2f8204b2a626b Mon Sep 17 00:00:00 2001 From: Father Chrysostomos Date: Tue, 24 Apr 2012 16:00:36 -0700 Subject: =?UTF-8?q?[perl=20#112358]=20Storable:=20Don=E2=80=99t=20create?= =?UTF-8?q?=20RV=20with=20no=20refcnt?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Otherwise assigning to it will cause the referent to be freed, because nothing but Storable knows that it has no reference count. Storable.xs was creating a new RV without increasing the refe- rence count on the referent. It was then using it to call the STORABLE_freeze method on the object. Since Perl passes arguments by reference, it was possible to unref the reference by assigning to $_[0] within STORABLE_freeze. That would cause the object’s reference count to go down. --- dist/Storable/Storable.xs | 3 +-- dist/Storable/t/blessed.t | 11 ++++++++++- 2 files changed, 11 insertions(+), 3 deletions(-) (limited to 'dist') diff --git a/dist/Storable/Storable.xs b/dist/Storable/Storable.xs index ca6f9b48ec..30f9281bb5 100644 --- a/dist/Storable/Storable.xs +++ b/dist/Storable/Storable.xs @@ -2916,9 +2916,8 @@ static int store_hook( TRACEME(("about to call STORABLE_freeze on class %s", classname)); - ref = newRV_noinc(sv); /* Temporary reference */ + ref = newRV_inc(sv); /* Temporary reference */ av = array_call(aTHX_ ref, hook, clone); /* @a = $object->STORABLE_freeze($c) */ - SvRV_set(ref, NULL); SvREFCNT_dec(ref); /* Reclaim temporary reference */ count = AvFILLp(av) + 1; diff --git a/dist/Storable/t/blessed.t b/dist/Storable/t/blessed.t index 6657e3c424..6b25f37270 100644 --- a/dist/Storable/t/blessed.t +++ b/dist/Storable/t/blessed.t @@ -27,7 +27,7 @@ use Storable qw(freeze thaw store retrieve); ); my $test = 12; -my $tests = $test + 22 + 2 * 6 * keys %::immortals; +my $tests = $test + 23 + 2 * 6 * keys %::immortals; plan(tests => $tests); package SHORT_NAME; @@ -249,3 +249,12 @@ is($STRESS_THE_STACK::freeze_count, 1); is($STRESS_THE_STACK::thaw_count, 1); isnt($t, undef); is(ref $t, 'STRESS_THE_STACK'); + +{ + package ModifyARG112358; + sub STORABLE_freeze { $_[0] = "foo"; } + my $o= {str=>bless {}}; + my $f= ::freeze($o); + ::is ref $o->{str}, __PACKAGE__, + 'assignment to $_[0] in STORABLE_freeze does not corrupt things'; +} -- cgit v1.2.1