diff options
author | Father Chrysostomos <sprout@cpan.org> | 2012-01-09 19:54:26 -0800 |
---|---|---|
committer | Father Chrysostomos <sprout@cpan.org> | 2012-01-09 19:54:26 -0800 |
commit | 60edcf09a5cb026822f99270a4bfbe3149cfbb52 (patch) | |
tree | 9651ece6289498ae99ca78bc4da131b5c22be3b6 /av.c | |
parent | f09f972badf5b299d86c10b00897652752e314ed (diff) | |
download | perl-60edcf09a5cb026822f99270a4bfbe3149cfbb52.tar.gz |
Better fix for perl #107440
> > Actually, the simplest solution seem to be to put the av or hv on
> > the mortals stack in pp_aassign and pp_undef, rather than in
> > [ah]v_undef/clear.
>
> This makes me nervous. The tmps stack is typically cleared only on
> statement boundaries, so we run the risks of
>
> * user-visible delaying of freeing elements;
> * large tmps stack growth might be possible with
> certain types of loop that repeatedly assign to an array without
> freeing tmps (eg map? I think I fixed most map/grep tmps leakage
> a
> while back, but there may still be some edge cases).
>
> Surely an ENTER/SAVEFREESV/LEAVE inside pp_aassign is just as
> efficient,
> without any attendant risks?
>
> Also, although pp_aassign and pp_undef are now fixed, the
> [ah]v_undef/clear functions aren't, and they're part of the public API
> that can be called independently of pp_aassign etc. Ideally they
> should
> be fixed (so they don't crash in mid-loop), and their documentation
> updated to point out that on return, their AV/HV arg may have been
> freed.
This commit takes care of the first part; it changes pp_aassign to use
ENTER/SAVEFREESV/LEAVE and adds the same to h_freeentries (called both
by hv_undef and hv_clear), av_undef and av_clear.
It effectively reverts the C code part of 9f71cfe6ef2.
Diffstat (limited to 'av.c')
-rw-r--r-- | av.c | 14 |
1 files changed, 11 insertions, 3 deletions
@@ -437,6 +437,7 @@ Perl_av_clear(pTHX_ register AV *av) { dVAR; I32 extra; + bool real; PERL_ARGS_ASSERT_AV_CLEAR; assert(SvTYPE(av) == SVt_PVAV); @@ -462,9 +463,11 @@ Perl_av_clear(pTHX_ register AV *av) if (AvMAX(av) < 0) return; - if (AvREAL(av)) { + if ((real = !!AvREAL(av))) { SV** const ary = AvARRAY(av); I32 index = AvFILLp(av) + 1; + ENTER; + SAVEFREESV(SvREFCNT_inc_simple_NN(av)); while (index) { SV * const sv = ary[--index]; /* undef the slot before freeing the value, because a @@ -479,7 +482,7 @@ Perl_av_clear(pTHX_ register AV *av) AvARRAY(av) = AvALLOC(av); } AvFILLp(av) = -1; - + if (real) LEAVE; } /* @@ -493,6 +496,8 @@ Undefines the array. Frees the memory used by the array itself. void Perl_av_undef(pTHX_ register AV *av) { + bool real; + PERL_ARGS_ASSERT_AV_UNDEF; assert(SvTYPE(av) == SVt_PVAV); @@ -500,8 +505,10 @@ Perl_av_undef(pTHX_ register AV *av) if (SvTIED_mg((const SV *)av, PERL_MAGIC_tied)) av_fill(av, -1); - if (AvREAL(av)) { + if ((real = !!AvREAL(av))) { register I32 key = AvFILLp(av) + 1; + ENTER; + SAVEFREESV(SvREFCNT_inc_simple_NN(av)); while (key) SvREFCNT_dec(AvARRAY(av)[--key]); } @@ -512,6 +519,7 @@ Perl_av_undef(pTHX_ register AV *av) AvMAX(av) = AvFILLp(av) = -1; if(SvRMAGICAL(av)) mg_clear(MUTABLE_SV(av)); + if(real) LEAVE; } /* |