develooper Front page | perl.perl5.porters | Postings from June 2016

Re: [perl #128313] Memory leak in perl 5.24.0 when use re qw[strict]is used

Thread Previous | Thread Next
From:
Dan Collins
Date:
June 8, 2016 16:27
Subject:
Re: [perl #128313] Memory leak in perl 5.24.0 when use re qw[strict]is used
Message ID:
CA+tt54+doF1OTF5NkYi2xae75cNSUfTUR79fA4md+mHKK9r_sA@mail.gmail.com
Vaguely, yes...but I realize that I may not have set  *posix_warnings =
warn_text;, and I may have accidentally commented out the whole line. :(
Does that pass tests for you?
On Jun 8, 2016 12:23 PM, "demerphq" <demerphq@gmail.com> wrote:

> On 8 June 2016 at 17:50, Dan Collins via RT <perlbug-followup@perl.org>
> wrote:
> > Confirmed in blead on linux.
> >
> > Valgrind with --leak-check=full --show-leak-kinds=all reports several
> records of the type:
> >
> >     ==9552== 7,339,416 bytes in 53,965 blocks are still reachable in
> loss record 651 of 651
> >     ==9552==    at 0x4C2AB5C: realloc (vg_replace_malloc.c:785)
> >     ==9552==    by 0x55BF69: Perl_safesysrealloc (util.c:274)
> >     ==9552==    by 0x5C83D1: Perl_sv_grow (sv.c:1602)
> >     ==9552==    by 0x5ED031: Perl_sv_catpvn_flags (sv.c:5414)
> >     ==9552==    by 0x60DD01: Perl_sv_vcatpvfn_flags (sv.c:11499)
> >     ==9552==    by 0x60CC6C: Perl_sv_vsetpvfn (sv.c:10834)
> >     ==9552==    by 0x604835: Perl_vnewSVpvf (sv.c:9448)
> >     ==9552==    by 0x604738: Perl_newSVpvf (sv.c:9433)
> >     ==9552==    by 0x52A809: S_handle_possible_posix (regcomp.c:14050)
> >     ==9552==    by 0x53246B: S_regclass (regcomp.c:15853)
> >     ==9552==    by 0x52287F: S_regatom (regcomp.c:12392)
> >     ==9552==    by 0x51D66D: S_regpiece (regcomp.c:11483)
> >
> > I was able to minimize the test case to:
> >
> >     use re qw[strict];
> >     my $s = 'aaa';
> >     my $ps = 'aa';
> >     while (1) {
> >         $s =~ /[^.]+$ps/;
> >     }
> >
> > The memory is allocated at 14050:
> >
> >     ADD_POSIX_WARNING(p, "there must be a starting ':'");
> >
> > The function will eventually return at 14498:
> >
> >     return NOT_MEANT_TO_BE_A_POSIX_CLASS;
> >
> > Leaving AV* warn_text not mortalized, since that doesn't happen until
> 14533:
> >
> >     if (posix_warnings) {
> >         /* mortalize to avoid a leak with FATAL warnings */
> >         *posix_warnings = (AV *) sv_2mortal((SV *) warn_text);
> >     }
> >     else {
> >         SvREFCNT_dec_NN(warn_text);
> >     }
> >
> > A possible solution is to mortalize warn_text at every location where
> S_handle_possible_posix can return NOT_MEANT_TO_BE_A_POSIX_CLASS. This is a
> bit ugly since I had to copy/paste the code to each of 6 locations, but
> it's the only way I could get `make test` to pass. At first, I tried making
> warn_text mortal when it's first allocated, but that seemed to make it get
> freed too early.
>
> Hrm. Did the patch you tried look like this?
>
> diff --git a/regcomp.c b/regcomp.c
> index 0b1e606..8562b8f 100644
> --- a/regcomp.c
> +++ b/regcomp.c
> @@ -13764,7 +13764,7 @@ S_populate_ANYOF_from_invlist(pTHX_ regnode
> *node, SV** invlist_ptr)
>   * routine. q.v. */
>  #define ADD_POSIX_WARNING(p, text)  STMT_START {
>   \
>          if (posix_warnings) {
>    \
> -            if (! warn_text) warn_text = newAV();
>    \
> +            if (! warn_text) warn_text = (AV *) sv_2mortal((SV *)
> newAV()); \
>              av_push(warn_text, Perl_newSVpvf(aTHX_
>   \
>                                               WARNING_PREFIX
>    \
>                                               text
>    \
> @@ -14530,13 +14530,9 @@ S_handle_possible_posix(pTHX_ RExC_state_t
> *pRExC_state,
>              }
>
>              if (warn_text) {
> -                if (posix_warnings) {
> -                    /* mortalize to avoid a leak with FATAL warnings */
> -                    *posix_warnings = (AV *) sv_2mortal((SV *) warn_text);
> -                }
> -                else {
> -                    SvREFCNT_dec_NN(warn_text);
> -                }
> +                /* warn_text should only be true if posix_warnings is
> true */
> +                assert(posix_warnings);
> +                *posix_warnings = warn_text;
>              }
>          }
>          else if (class_number != OOB_NAMEDCLASS) {
>
>
> Yves
>

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About