Skip Menu |
Report information
Id: 128313
Status: resolved
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: sc [at] charnell.plus.com
zdm [at] softvisio.net
Cc:
AdminCc:

Operating System: mswin32
PatchStatus: (no value)
Severity: High
Type: core
Perl Version: 5.24.0
Fixed In: (no value)

Attachments
0001-perl-128313-Fix-leak-in-perl-5.24-with-strict-and-re.patch
0001-perl-128313-Fix-memory-leak-in-POSIX-class-warnings.patch
0002-perl-128313-Fix-memory-leak-in-POSIX-class-warnings.patch
0003-perl-128313-test-for-memory-leak-in-POSIX-classes.patch



Date: Fri, 3 Jun 2016 14:07:13 +0300
Subject: Memory leak in perl 5.24.0 when use re qw[strict] is used
From: "zdm [...] softvisio.net" <zdm [...] softvisio.net>
To: perlbug [...] perl.org
Download (untitled) / with headers
text/plain 3.4k
Reply-To: zdm@softvisio.net From: zdm@softvisio.net Message-Id: <5.24.0_3968_1464951615@WIN-OPJJ674O0JK> To: perlbug@perl.org Subject: Memory leak in perl 5.24.0 when use re qw[strict] is used This is a bug report for perl from zdm@softvisio.net, generated with the help of perlbug 1.40 running under perl 5.24.0. ----------------------------------------------------------------- Hi. This code produce memory leak: use re qw[strict]; my $s = 'aaa'; while (1) { my $ps = 'aa'; $s =~ /[^.]+\.$ps\z/sm; } If I remove "use re qw[strict];" - mem. leak disappearing. [Please do not change anything below this line] ----------------------------------------------------------------- --- Flags: category=core severity=high --- Site configuration information for perl 5.24.0: Configured by strawberry-perl at Tue May 10 21:33:22 2016. Summary of my perl5 (revision 5 version 24 subversion 0) configuration: Platform: osname=MSWin32, osvers=6.3, archname=MSWin32-x64-multi-thread uname='Win32 strawberry-perl 5.24.0.1 #1 Tue May 10 21:30:49 2016 x64' config_args='undef' hint=recommended, useposix=true, d_sigaction=undef useithreads=define, usemultiplicity=define use64bitint=define, use64bitall=undef, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='gcc', ccflags =' -s -O2 -DWIN32 -DWIN64 -DCONSERVATIVE -DPERL_TEXTMODE_SCRIPTS -DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS -fwrapv -fno-strict-aliasing -mms-bitfields', optimize='-s -O2', cppflags='-DWIN32' ccversion='', gccversion='4.9.2', gccosandvers='' intsize=4, longsize=4, ptrsize=8, doublesize=8, byteorder=12345678, doublekind=3 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16, longdblkind=3 ivtype='long long', ivsize=8, nvtype='double', nvsize=8, Off_t='long long', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='g++.exe', ldflags ='-s -L"D:\devel\perl\perl\lib\CORE" -L"D:\devel\perl\c\lib"' libpth=D:\devel\perl\c\lib D:\devel\perl\c\x86_64-w64-mingw32\lib D:\devel\perl\c\lib\gcc\x86_64-w64-mingw32\4.9.2 libs=-lmoldname -lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32 -ladvapi32 -lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr -lwinmm -lversion -lodbc32 -lodbccp32 -lcomctl32 perllibs=-lmoldname -lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32 -ladvapi32 -lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr -lwinmm -lversion -lodbc32 -lodbccp32 -lcomctl32 libc=, so=dll, useshrplib=true, libperl=libperl524.a gnulibc_version='' Dynamic Linking: dlsrc=dl_win32.xs, dlext=xs.dll, d_dlsymun=undef, ccdlflags=' ' cccdlflags=' ', lddlflags='-mdll -s -L"D:\devel\perl\perl\lib\CORE" -L"D:\devel\perl\c\lib"' --- @INC for perl 5.24.0: d:/projects/pcore/lib d:/devel/perl/perl/site/lib/MSWin32-x64-multi-thread d:/devel/perl/perl/site/lib d:/devel/perl/perl/vendor/lib d:/devel/perl/perl/lib . --- Environment for perl 5.24.0: HOME (unset) LANG=en_US LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=C:\Windows;C:\Windows\system32;C:\Windows\system32\wbem;C:\Windows\System32\WindowsPowerShell\v1.0;d:\apps\bin;d:\apps\gpg;d:\devel\bin;d:\devel\perl\perl\site\bin;d:\devel\perl\perl\bin;d:\devel\perl\c\bin;d:\devel\hg;d:\devel\jre\bin;d:\devel\sencha\sencha-cmd;d:\devel\nodejs;d:\devel\msys2\usr\bin;d:\projects\pcore\bin;;d:/data PERL5LIB=d:/projects/pcore/lib; PERL_BADLANG (unset) SHELL (unset)
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.9k
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. No idea how to write a test for this.
Subject: 0001-perl-128313-Fix-memory-leak-in-POSIX-class-warnings.patch
From e39d6012d7aafe4de01afea5c32965e69936dd76 Mon Sep 17 00:00:00 2001 From: Dan Collins <dcollinsn@gmail.com> Date: Wed, 8 Jun 2016 11:26:29 -0400 Subject: [PATCH] [perl #128313] Fix memory leak in POSIX class warnings Certain classes that "may be" POSIX classes result in POSIX warnings being added to warn_text, but never freed, resulting in a slow but present memory leak. We need to mortalize warn_text. warn_text is presently mortalized or freed late in the function, just before it would return successfully. However, there are a number of points where this function can return failure. If a POSIX warning is generated and the function returns before warn_text can be made mortal, it will never be freed. --- regcomp.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/regcomp.c b/regcomp.c index 0b1e606..0566d0e 100644 --- a/regcomp.c +++ b/regcomp.c @@ -14072,6 +14072,9 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state, /* We consider something like [^:^alnum:]] to not have been intended to * be a posix class, but XXX maybe we should */ if (complement) { + if (warn_text) { + warn_text = (AV *)sv_2mortal((SV *) warn_text); + } return NOT_MEANT_TO_BE_A_POSIX_CLASS; } @@ -14099,6 +14102,9 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state, * this leaves this construct looking like [:] or [:^], which almost * certainly weren't intended to be posix classes */ if (has_opening_bracket) { + if (warn_text) { + warn_text = (AV *)sv_2mortal((SV *) warn_text); + } return NOT_MEANT_TO_BE_A_POSIX_CLASS; } @@ -14116,6 +14122,9 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state, /* XXX We are currently very restrictive here, so this code doesn't * consider the possibility that, say, /[alpha.]]/ was intended to * be a posix class. */ + if (warn_text) { + warn_text = (AV *)sv_2mortal((SV *) warn_text); + } return NOT_MEANT_TO_BE_A_POSIX_CLASS; } @@ -14286,6 +14295,9 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state, } /* Otherwise, it can't have meant to have been a class */ + if (warn_text) { + warn_text = (AV *)sv_2mortal((SV *) warn_text); + } return NOT_MEANT_TO_BE_A_POSIX_CLASS; } @@ -14336,6 +14348,9 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state, * class name. (We can do this on the first pass, as any second pass * will yield an even shorter name) */ if (name_len < 3) { + if (warn_text) { + warn_text = (AV *)sv_2mortal((SV *) warn_text); + } return NOT_MEANT_TO_BE_A_POSIX_CLASS; } @@ -14495,6 +14510,9 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state, } /* Here neither pass found a close-enough class name */ + if (warn_text) { + warn_text = (AV *)sv_2mortal((SV *) warn_text); + } return NOT_MEANT_TO_BE_A_POSIX_CLASS; } -- 2.8.1
Date: Wed, 8 Jun 2016 18:23:09 +0200
From: demerphq <demerphq [...] gmail.com>
Subject: Re: [perl #128313] Memory leak in perl 5.24.0 when use re qw[strict] is used
CC: Perl5 Porteros <perl5-porters [...] perl.org>
To: Perl RT Bug Tracker <perlbug-followup [...] perl.org>
Download (untitled) / with headers
text/plain 3.5k
On 8 June 2016 at 17:50, Dan Collins via RT <perlbug-followup@perl.org> wrote: Show quoted text
> 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
Date: Wed, 8 Jun 2016 12:27:26 -0400
CC: Perl5 Porters <perl5-porters [...] perl.org>, perlbug-followup [...] perl.org
To: demerphq <demerphq [...] gmail.com>
Subject: Re: [perl #128313] Memory leak in perl 5.24.0 when use re qw[strict] is used
From: Dan Collins <dcollinsn [...] gmail.com>
Download (untitled) / with headers
text/plain 3.7k

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:
Show quoted text
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
Subject: Re: [perl #128313] Memory leak in perl 5.24.0 when use re qw[strict] is used
From: demerphq <demerphq [...] gmail.com>
To: Dan Collins <dcollinsn [...] gmail.com>
CC: Perl5 Porters <perl5-porters [...] perl.org>, Perl RT Bug Tracker <perlbug-followup [...] perl.org>
Date: Wed, 8 Jun 2016 18:44:23 +0200
Download (untitled) / with headers
text/plain 631b
On 8 June 2016 at 18:27, Dan Collins <dcollinsn@gmail.com> wrote: Show quoted text
> 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?
Yes it passed all tests here. I pushed it just now: commit ee072c898947f5fee316f1381b29ad692addcf05 Author: Yves Orton <demerphq@gmail.com> Date: Wed Jun 8 18:42:30 2016 +0200 [perl #128313] Fix leak in perl 5.24 with strict and regex posix char classes This patch is a refinement of one written by Dan Collins. Any thanks for this patch should go to him. Yves
Date: Wed, 8 Jun 2016 11:02:19 -0600
Subject: Re: [perl #128313] Memory leak in perl 5.24.0 when use re qw[strict] is used
From: Karl Williamson <public [...] khwilliamson.com>
To: demerphq <demerphq [...] gmail.com>, Dan Collins <dcollinsn [...] gmail.com>
CC: Perl5 Porters <perl5-porters [...] perl.org>, Perl RT Bug Tracker <perlbug-followup [...] perl.org>
Download (untitled) / with headers
text/plain 1.4k
On 06/08/2016 10:44 AM, demerphq wrote: Show quoted text
> On 8 June 2016 at 18:27, Dan Collins <dcollinsn@gmail.com> wrote:
>> 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?
> > Yes it passed all tests here. > > I pushed it just now: > > commit ee072c898947f5fee316f1381b29ad692addcf05 > Author: Yves Orton <demerphq@gmail.com> > Date: Wed Jun 8 18:42:30 2016 +0200 > > [perl #128313] Fix leak in perl 5.24 with strict and regex posix > char classes > > This patch is a refinement of one written by Dan Collins. > > Any thanks for this patch should go to him. > > > Yves >
I'm concerned that you guys are falling into the same trap I did earlier. The problem with mortalizing is that it can lead to excessive memory use, as those temps don't get freed immediately. See https://rt.perl.org/Ticket/Display.html?id=127392 In a large pattern, this can be called over and over and over. It may not be a problem, but it might be, and that needs to be considered. This doesn't appear to have the potential to use large amounts of memory per scalar like what was reported in that ticket. So it may be ok. If it isn't ok, one option is to add this mortalization only if it's re 'strict'. Another would be to make the return into a macro that frees the space before returning. This would solve the 'ugliness' issue.
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 701b
Yves' patch changes warn_text from leaking to mortal if posix_warnings is set and the function ends early, no change in any other case. In other words, it /never/ called SvREFCNT_dec_NN(warn_text), either before or now. Alternately, since posix_warnings isn't set if the function returns early, we can SvREFCNT_dec(warn_text) if we return early. My limited testing suggests that the memory advantage is minimal, but it's probably "more right" to free it immediately before returning. Is the attached patch what you were thinking? It still fixes this bug, and all tests still pass. (Patch is against blead /before/ Yves' patch was applied, if you'd prefer a patch against blead, that can be arranged.
Subject: 0002-perl-128313-Fix-memory-leak-in-POSIX-class-warnings.patch
From b04aa5cec7c85fa03e65b7dd86394d818a3233eb Mon Sep 17 00:00:00 2001 From: Dan Collins <dcollinsn@gmail.com> Date: Wed, 8 Jun 2016 14:26:05 -0400 Subject: [PATCH] [perl #128313] Fix memory leak in POSIX class warnings Certain classes that "may be" POSIX classes result in POSIX warnings being added to warn_text, but never freed, resulting in a slow but present memory leak. We need to ensure that warn_text is freed. warn_text is presently mortalized late in the function, when it is assigned to *posix_warnings. However, certain cases can generate a POSIX warning while also having the function return before that point. If a POSIX warning is generated and the function returns before warn_text can be made mortal, it will never be freed. This patch performs a REFCNT_dec on warn_text immediately before any early return. --- regcomp.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/regcomp.c b/regcomp.c index 0b1e606..6bd903d 100644 --- a/regcomp.c +++ b/regcomp.c @@ -13772,6 +13772,10 @@ S_populate_ANYOF_from_invlist(pTHX_ regnode *node, SV** invlist_ptr) REPORT_LOCATION_ARGS(p))); \ } \ } STMT_END +#define RETURN_EARLY(retval) STMT_START { \ + if (warn_text) SvREFCNT_dec_NN(warn_text); \ + return (retval); \ + } STMT_END STATIC int S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state, @@ -13913,7 +13917,7 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state, PERL_ARGS_ASSERT_HANDLE_POSSIBLE_POSIX; if (p >= e) { - return NOT_MEANT_TO_BE_A_POSIX_CLASS; + RETURN_EARLY(NOT_MEANT_TO_BE_A_POSIX_CLASS); } if (*(p - 1) != '[') { @@ -14002,7 +14006,7 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state, *updated_parse_ptr = (char *) temp_ptr; } - return OOB_NAMEDCLASS; + RETURN_EARLY(OOB_NAMEDCLASS); } } @@ -14072,7 +14076,7 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state, /* We consider something like [^:^alnum:]] to not have been intended to * be a posix class, but XXX maybe we should */ if (complement) { - return NOT_MEANT_TO_BE_A_POSIX_CLASS; + RETURN_EARLY(NOT_MEANT_TO_BE_A_POSIX_CLASS); } complement = 1; @@ -14099,7 +14103,7 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state, * this leaves this construct looking like [:] or [:^], which almost * certainly weren't intended to be posix classes */ if (has_opening_bracket) { - return NOT_MEANT_TO_BE_A_POSIX_CLASS; + RETURN_EARLY(NOT_MEANT_TO_BE_A_POSIX_CLASS); } /* But this function can be called when we parse the colon for @@ -14116,7 +14120,7 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state, /* XXX We are currently very restrictive here, so this code doesn't * consider the possibility that, say, /[alpha.]]/ was intended to * be a posix class. */ - return NOT_MEANT_TO_BE_A_POSIX_CLASS; + RETURN_EARLY(NOT_MEANT_TO_BE_A_POSIX_CLASS); } /* Here we have something like 'foo:]'. There was no initial colon, @@ -14286,7 +14290,7 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state, } /* Otherwise, it can't have meant to have been a class */ - return NOT_MEANT_TO_BE_A_POSIX_CLASS; + RETURN_EARLY(NOT_MEANT_TO_BE_A_POSIX_CLASS); } /* If we ran off the end, and the final character was a punctuation @@ -14336,7 +14340,7 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state, * class name. (We can do this on the first pass, as any second pass * will yield an even shorter name) */ if (name_len < 3) { - return NOT_MEANT_TO_BE_A_POSIX_CLASS; + RETURN_EARLY(NOT_MEANT_TO_BE_A_POSIX_CLASS); } /* Find which class it is. Initially switch on the length of the name. @@ -14495,7 +14499,7 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state, } /* Here neither pass found a close-enough class name */ - return NOT_MEANT_TO_BE_A_POSIX_CLASS; + RETURN_EARLY(NOT_MEANT_TO_BE_A_POSIX_CLASS); } probably_meant_to_be: @@ -14530,13 +14534,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 is only true if posix_warnings is true */ + assert(posix_warnings); + *posix_warnings = (AV *) sv_2mortal((SV *) warn_text); } } else if (class_number != OOB_NAMEDCLASS) { @@ -14562,6 +14562,7 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state, return OOB_NAMEDCLASS; } #undef ADD_POSIX_WARNING +#undef RETURN_EARLY STATIC unsigned int S_regex_set_precedence(const U8 my_operator) { -- 2.8.1
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 141b
On Wed Jun 08 08:50:49 2016, dcollinsn@gmail.com wrote: Show quoted text
> No idea how to write a test for this.
See t/op/svleak.t -- Father Chrysostomos
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 504b
Indeed. This test will work with any proposed fix for this patch, and is confirmed failing on blead before Yves' commit, confirmed passing on blead after Yves' commit, and confirmed passing with my patch 0002. So I believe that the best course of action, unless khw believes that this is still likely to increase memory usage significantly in some cases, is to revert ee072c898947f5fee316f1381b29ad692addcf05 and apply 0002 and 0003 - unless there's an argument that ee072c898 is better than 0002.patch.
Subject: 0003-perl-128313-test-for-memory-leak-in-POSIX-classes.patch
From 1e1c9128aaf421762f66498ed90fecc9621976da Mon Sep 17 00:00:00 2001 From: Dan Collins <dcollinsn@gmail.com> Date: Wed, 8 Jun 2016 16:26:07 -0400 Subject: [PATCH] [perl #128313] test for memory leak in POSIX classes --- t/op/svleak.t | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/t/op/svleak.t b/t/op/svleak.t index 595bf3e..eac20fb 100644 --- a/t/op/svleak.t +++ b/t/op/svleak.t @@ -15,7 +15,7 @@ BEGIN { use Config; -plan tests => 131; +plan tests => 132; # run some code N times. If the number of SVs at the end of loop N is # greater than (N-1)*delta at the end of loop 1, we've got a leak @@ -537,3 +537,14 @@ EOF ::leak(5, 0, \&f, q{goto shouldn't leak @_}); } + +# [perl #128313] POSIX warnings shouldn't leak +{ + no warnings 'experimental'; + use re 'strict'; + my $a = 'aaa'; + my $b = 'aa'; + sub f { $a =~ /[^.]+$b/; } + ::leak(2, 0, \&f, q{use re 'strict' shouldn't leak warning strings}); +} -- 2.8.1
From: Karl Williamson <public [...] khwilliamson.com>
Subject: Re: [perl #128313] Memory leak in perl 5.24.0 when use re qw[strict] is used
To: perlbug-followup [...] perl.org
CC: perl5-porters [...] perl.org
Date: Wed, 8 Jun 2016 15:14:42 -0600
Download (untitled) / with headers
text/plain 920b
On 06/08/2016 02:38 PM, Dan Collins via RT wrote: Show quoted text
> Indeed. This test will work with any proposed fix for this patch, and is confirmed failing on blead before Yves' commit, confirmed passing on blead after Yves' commit, and confirmed passing with my patch 0002. > > So I believe that the best course of action, unless khw believes that this is still likely to increase memory usage significantly in some cases, is to revert ee072c898947f5fee316f1381b29ad692addcf05 and apply 0002 and 0003 - unless there's an argument that ee072c898 is better than 0002.patch.
Notice that my email just said 'concerned'. I'm not sure the memory issue is a real problem. But, since the patch is already written, and solves the problem once and for all, I think your action proposal is correct, but lets wait to hear from Yves. Show quoted text
> > --- > via perlbug: queue: perl5 status: open > https://rt.perl.org/Ticket/Display.html?id=128313 >
To: Perl RT Bug Tracker <perlbug-followup [...] perl.org>
From: demerphq <demerphq [...] gmail.com>
Subject: Re: [perl #128313] Memory leak in perl 5.24.0 when use re qw[strict] is used
Date: Thu, 9 Jun 2016 10:14:32 +0200
Download (untitled) / with headers
text/plain 6.6k


On 8 Jun 2016 20:38, "Dan Collins via RT" <perlbug-followup@perl.org> wrote:
Show quoted text

>
> Yves' patch changes warn_text from leaking to mortal if posix_warnings is set and the function ends early, no change in any other case. In other words, it /never/ called SvREFCNT_dec_NN(warn_text), either before or now.
>
> Alternately, since posix_warnings isn't set if the function returns early, we can SvREFCNT_dec(warn_text) if we return early. My limited testing suggests that the memory advantage is minimal, but it's probably "more right" to free it immediately before returning. Is the attached patch what you were thinking? It still fixes this bug, and all tests still pass.
>
> (Patch is against blead /before/ Yves' patch was applied, if you'd prefer a patch against blead, that can be arranged.

Wait, wait. :-)

Seems to me that if this is a real concern, and i trust karls judgement there although i admit to being surprised, then this av should just become part of the RExC struct and it should be reused each time.

Cheers
Yves

>
Show quoted text

> ---
> via perlbug:  queue: perl5 status: open
> https://rt.perl.org/Ticket/Display.html?id=128313
>
> From b04aa5cec7c85fa03e65b7dd86394d818a3233eb Mon Sep 17 00:00:00 2001
> From: Dan Collins <dcollinsn@gmail.com>
> Date: Wed, 8 Jun 2016 14:26:05 -0400
> Subject: [PATCH] [perl #128313] Fix memory leak in POSIX class warnings
>
> Certain classes that "may be" POSIX classes result in POSIX warnings
> being added to warn_text, but never freed, resulting in a slow but
> present memory leak. We need to ensure that warn_text is freed.
>
> warn_text is presently mortalized late in the function, when it is
> assigned to *posix_warnings. However, certain cases can generate
> a POSIX warning while also having the function return before that
> point. If a POSIX warning is generated and the function returns
> before warn_text can be made mortal, it will never be freed.
>
> This patch performs a REFCNT_dec on warn_text immediately before
> any early return.
> ---
>  regcomp.c | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/regcomp.c b/regcomp.c
> index 0b1e606..6bd903d 100644
> --- a/regcomp.c
> +++ b/regcomp.c
> @@ -13772,6 +13772,10 @@ S_populate_ANYOF_from_invlist(pTHX_ regnode *node, SV** invlist_ptr)
>                                               REPORT_LOCATION_ARGS(p)));     \
>          }                                                                   \
>      } STMT_END
> +#define RETURN_EARLY(retval) STMT_START {                                   \
> +        if (warn_text) SvREFCNT_dec_NN(warn_text);                          \
> +        return (retval);                                                    \
> +    } STMT_END
>
>  STATIC int
>  S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state,
> @@ -13913,7 +13917,7 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state,
>      PERL_ARGS_ASSERT_HANDLE_POSSIBLE_POSIX;
>
>      if (p >= e) {
> -        return NOT_MEANT_TO_BE_A_POSIX_CLASS;
> +        RETURN_EARLY(NOT_MEANT_TO_BE_A_POSIX_CLASS);
>      }
>
>      if (*(p - 1) != '[') {
> @@ -14002,7 +14006,7 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state,
>                      *updated_parse_ptr = (char *) temp_ptr;
>                  }
>
> -                return OOB_NAMEDCLASS;
> +                RETURN_EARLY(OOB_NAMEDCLASS);
>              }
>          }
>
> @@ -14072,7 +14076,7 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state,
>          /* We consider something like [^:^alnum:]] to not have been intended to
>           * be a posix class, but XXX maybe we should */
>          if (complement) {
> -            return NOT_MEANT_TO_BE_A_POSIX_CLASS;
> +            RETURN_EARLY(NOT_MEANT_TO_BE_A_POSIX_CLASS);
>          }
>
>          complement = 1;
> @@ -14099,7 +14103,7 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state,
>           * this leaves this construct looking like [:] or [:^], which almost
>           * certainly weren't intended to be posix classes */
>          if (has_opening_bracket) {
> -            return NOT_MEANT_TO_BE_A_POSIX_CLASS;
> +            RETURN_EARLY(NOT_MEANT_TO_BE_A_POSIX_CLASS);
>          }
>
>          /* But this function can be called when we parse the colon for
> @@ -14116,7 +14120,7 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state,
>              /* XXX We are currently very restrictive here, so this code doesn't
>               * consider the possibility that, say, /[alpha.]]/ was intended to
>               * be a posix class. */
> -            return NOT_MEANT_TO_BE_A_POSIX_CLASS;
> +            RETURN_EARLY(NOT_MEANT_TO_BE_A_POSIX_CLASS);
>          }
>
>          /* Here we have something like 'foo:]'.  There was no initial colon,
> @@ -14286,7 +14290,7 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state,
>              }
>
>              /* Otherwise, it can't have meant to have been a class */
> -            return NOT_MEANT_TO_BE_A_POSIX_CLASS;
> +            RETURN_EARLY(NOT_MEANT_TO_BE_A_POSIX_CLASS);
>          }
>
>          /* If we ran off the end, and the final character was a punctuation
> @@ -14336,7 +14340,7 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state,
>           * class name.  (We can do this on the first pass, as any second pass
>           * will yield an even shorter name) */
>          if (name_len < 3) {
> -            return NOT_MEANT_TO_BE_A_POSIX_CLASS;
> +            RETURN_EARLY(NOT_MEANT_TO_BE_A_POSIX_CLASS);
>          }
>
>          /* Find which class it is.  Initially switch on the length of the name.
> @@ -14495,7 +14499,7 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state,
>              }
>
>              /* Here neither pass found a close-enough class name */
> -            return NOT_MEANT_TO_BE_A_POSIX_CLASS;
> +            RETURN_EARLY(NOT_MEANT_TO_BE_A_POSIX_CLASS);
>          }
>
>      probably_meant_to_be:
> @@ -14530,13 +14534,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 is only true if posix_warnings is true */
> +                assert(posix_warnings);
> +                *posix_warnings = (AV *) sv_2mortal((SV *) warn_text);
>              }
>          }
>          else if (class_number != OOB_NAMEDCLASS) {
> @@ -14562,6 +14562,7 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state,
>      return OOB_NAMEDCLASS;
>  }
>  #undef ADD_POSIX_WARNING
> +#undef RETURN_EARLY
>
>  STATIC unsigned  int
>  S_regex_set_precedence(const U8 my_operator) {
> --
> 2.8.1
>
>

Date: Thu, 9 Jun 2016 10:21:31 +0200
From: demerphq <demerphq [...] gmail.com>
Subject: Re: [perl #128313] Memory leak in perl 5.24.0 when use re qw[strict] is used
CC: Perl RT Bug Tracker <perlbug-followup [...] perl.org>, Perl5 Porteros <perl5-porters [...] perl.org>
To: karl williamson <public [...] khwilliamson.com>


On 9 Jun 2016 00:06, "Karl Williamson" <public@khwilliamson.com> wrote:
Show quoted text

>
> On 06/08/2016 02:38 PM, Dan Collins via RT wrote:
>>
>> Indeed. This test will work with any proposed fix for this patch, and is confirmed failing on blead before Yves' commit, confirmed passing on blead after Yves' commit, and confirmed passing with my patch 0002.
>>
>> So I believe that the best course of action, unless khw believes that this is still likely to increase memory usage significantly in some cases, is to revert ee072c898947f5fee316f1381b29ad692addcf05 and apply 0002 and 0003 - unless there's an argument that ee072c898 is better than 0002.patch.
>
>
> Notice that my email just said 'concerned'.  I'm not sure the memory issue is a real problem.  But, since the patch is already written, and solves the problem once and for all, I think your action proposal is correct, but lets wait to hear from Yves.

I think a better solution is to put warn_text in RExC struct and reuse the same av over and over. I will put togethdr a patch for review.

Yves

Date: Fri, 10 Jun 2016 13:32:15 +0200
To: karl williamson <public [...] khwilliamson.com>
CC: Perl RT Bug Tracker <perlbug-followup [...] perl.org>, Perl5 Porteros <perl5-porters [...] perl.org>
Subject: Re: [perl #128313] Memory leak in perl 5.24.0 when use re qw[strict] is used
From: demerphq <demerphq [...] gmail.com>
Download (untitled) / with headers
text/plain 1.2k
Pushed as 7eec73eb790f7c4982edfc28c17c011e8a072490 Cheers, Yves On 9 June 2016 at 10:21, demerphq <demerphq@gmail.com> wrote: Show quoted text
> > On 9 Jun 2016 00:06, "Karl Williamson" <public@khwilliamson.com> wrote:
>> >> On 06/08/2016 02:38 PM, Dan Collins via RT wrote:
>>> >>> Indeed. This test will work with any proposed fix for this patch, and is >>> confirmed failing on blead before Yves' commit, confirmed passing on blead >>> after Yves' commit, and confirmed passing with my patch 0002. >>> >>> So I believe that the best course of action, unless khw believes that >>> this is still likely to increase memory usage significantly in some cases, >>> is to revert ee072c898947f5fee316f1381b29ad692addcf05 and apply 0002 and >>> 0003 - unless there's an argument that ee072c898 is better than 0002.patch.
>> >> >> Notice that my email just said 'concerned'. I'm not sure the memory issue >> is a real problem. But, since the patch is already written, and solves the >> problem once and for all, I think your action proposal is correct, but lets >> wait to hear from Yves.
> > I think a better solution is to put warn_text in RExC struct and reuse the > same av over and over. I will put togethdr a patch for review. > > Yves
-- perl -Mre=debug -e "/just|another|perl|hacker/"
Date: Fri, 10 Jun 2016 13:35:31 +0200
To: karl williamson <public [...] khwilliamson.com>
CC: Perl RT Bug Tracker <perlbug-followup [...] perl.org>, Perl5 Porteros <perl5-porters [...] perl.org>
From: demerphq <demerphq [...] gmail.com>
Subject: Re: [perl #128313] Memory leak in perl 5.24.0 when use re qw[strict] is used
Download (untitled) / with headers
text/plain 181b
On 10 June 2016 at 13:32, demerphq <demerphq@gmail.com> wrote: Show quoted text
> Pushed as 7eec73eb790f7c4982edfc28c17c011e8a072490
And amended by 0bf54b1ecaec8f6d80845d6cb77d62f8c9f4c415 Yves
Date: Mon, 13 Jun 2016 08:18:34 -0400
CC: Perl RT Bug Tracker <perlbug-followup [...] perl.org>, Perl5 Porteros <perl5-porters [...] perl.org>
From: Dan Collins <dcollinsn [...] gmail.com>
Subject: Re: [perl #128313] Memory leak in perl 5.24.0 when use re qw[strict] is used
Download (untitled) / with headers
text/plain 398b
Can you please also consider and apply the test for this bug, which is (I believe) attachment 3 to this ticket, before closing this?

On Fri, Jun 10, 2016 at 7:35 AM, demerphq <demerphq@gmail.com> wrote:
Show quoted text
On 10 June 2016 at 13:32, demerphq <demerphq@gmail.com> wrote:
> Pushed as 7eec73eb790f7c4982edfc28c17c011e8a072490


And amended by 0bf54b1ecaec8f6d80845d6cb77d62f8c9f4c415

Yves

RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 252b
On Mon Jun 13 05:19:21 2016, dcollinsn@gmail.com wrote: Show quoted text
> Can you please also consider and apply the test for this bug, which is (I > believe) attachment 3 to this ticket, before closing this?
Applied as 222c4b0. Thank you. -- Father Chrysostomos
From: sc [...] charnell.plus.com
Subject: Suspected memory leak with match operation
Date: Wed, 20 Jul 2016 15:36:58 +0100 (BST)
To: perlbug [...] perl.org
Download (untitled) / with headers
text/plain 5.4k
This is a bug report for perl from sc@charnell.plus.com, generated with the help of perlbug 1.40 running under perl 5.24.0. ----------------------------------------------------------------- A perl script I use to parse the XML produced from a Current cost energy meter looks to have a memory leak after I updated from perl 5.22.0 to perl 5.24.0. The script runs continuously and when running with perl 5.24.0 its memory footprint increases markedly over time. The memory footprint did not increase with perl 5.22.0. I have tried the script on armv5tel & x86_64 platforms running archlinux and both platforms exhibit the issue. I think I have narrowed the issue down to a string match operation and have developed a test-case, the limit used in the 'for' loop in the main subroutine can be increased to allow the script to run for long enough for the issue to be apparent. The perl program from which the test-case was taken was written a long while ago, so my perl may not be the best. The test-case follows: #!/usr/bin/perl -w use strict; use warnings; use POSIX; my $sensors = 8; # 9 sensors, 0..8 my $line; my @test_data; my $test_data = ("<msg><src>CC128-v1.48</src><dsb>00860</dsb><time>11:28:22</time><tmpr>19.0</tmpr><sensor>2</sensor><id>01210</id><type>1</type><ch1><watts>00101</watts></ch1></msg> <msg><src>CC128-v1.48</src><dsb>00898</dsb><time>17:16:30</time><hist><dsw>00900</dsw><type>1</type><units>kwhr</units><data><sensor>0</sensor><h006>0.000</h006><h004>0.046</h004></data></hist></msg> "); sub simple_sub { my ($line) = @_; my $sensor; my $sep = "h"; my $found = 0; for ($sensor = 0; $sensor <= $sensors; $sensor++) { # 2 values per sensor if ($line =~ m!<time>(\d+):(\d+):(\d+)</time><hist>.*<data><sensor>$sensor</sensor><$sep(\d+)>([\-\d.]+)</$sep(\d+)><$sep(\d+)>([\-\d.]+)</$sep(\d+)></data>!) { $found = 1; } } return; } sub main { my $line; my $loop; @test_data = split /\n/, $test_data; foreach $line (@test_data) { for ($loop = 0; $loop < 100000; $loop++) { simple_sub($line); } } return; } main(); exit [Please do not change anything below this line] ----------------------------------------------------------------- --- Flags: category=core severity=medium --- Site configuration information for perl 5.24.0: Configured by builduser at Sun Jun 5 09:49:52 MDT 2016. Summary of my perl5 (revision 5 version 24 subversion 0) configuration: Platform: osname=linux, osvers=3.10.96-3-arch, archname=armv5tel-linux-thread-multi uname='linux leming 3.10.96-3-arch #1 smp preempt thu mar 24 19:24:55 mdt 2016 armv5tel gnulinux ' config_args='-des -Dusethreads -Duseshrplib -Doptimize=-march=armv5te -O2 -pipe -fstack-protector --param=ssp-buffer-size=4 -Dprefix=/usr -Dvendorprefix=/usr -Dprivlib=/usr/share/perl5/core_perl -Darchlib=/usr/lib/perl5/core_perl -Dsitelib=/usr/share/perl5/site_perl -Dsitearch=/usr/lib/perl5/site_perl -Dvendorlib=/usr/share/perl5/vendor_perl -Dvendorarch=/usr/lib/perl5/vendor_perl -Dscriptdir=/usr/bin/core_perl -Dsitescript=/usr/bin/site_perl -Dvendorscript=/usr/bin/vendor_perl -Dinc_version_list=none -Dman1ext=1perl -Dman3ext=3perl -Dlddlflags=-shared -Wl,-O1,--sort-common,--as-needed,-z,relro -Dldflags=-Wl,-O1,--sort-common,--as-needed,-z,relro' hint=recommended, useposix=true, d_sigaction=define useithreads=define, usemultiplicity=define use64bitint=undef, use64bitall=undef, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', optimize='-march=armv5te -O2 -pipe -fstack-protector --param=ssp-buffer-size=4', cppflags='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include' ccversion='', gccversion='6.1.1 20160501', gccosandvers='' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234, doublekind=3 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=8, longdblkind=0 ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='cc', ldflags ='-Wl,-O1,--sort-common,--as-needed,-z,relro -fstack-protector-strong -L/usr/local/lib' libpth=/usr/local/lib /usr/lib/gcc/armv5tel-unknown-linux-gnueabi/6.1.1/include-fixed /usr/lib /lib libs=-lpthread -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc libc=libc-2.23.so, so=so, useshrplib=true, libperl=libperl.so gnulibc_version='2.23' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E -Wl,-rpath,/usr/lib/perl5/core_perl/CORE' cccdlflags='-fPIC', lddlflags='-shared -Wl,-O1,--sort-common,--as-needed,-z,relro -L/usr/local/lib -fstack-protector-strong' --- @INC for perl 5.24.0: /usr/lib/perl5/site_perl /usr/share/perl5/site_perl /usr/lib/perl5/vendor_perl /usr/share/perl5/vendor_perl /usr/lib/perl5/core_perl /usr/share/perl5/core_perl . --- Environment for perl 5.24.0: HOME=/root LANG=en_GB.utf8 LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/usr/local/sbin:/usr/local/bin:/usr/bin:/usr/bin/site_perl:/usr/bin/vendor_perl:/usr/bin/core_perl PERL_BADLANG (unset) SHELL=/bin/bash
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 681b
On Wed Jul 20 07:37:14 2016, sc@charnell.plus.com wrote: Show quoted text
> A perl script I use to parse the XML produced from a Current cost > energy meter looks to have a memory leak > after I updated from perl 5.22.0 to perl 5.24.0. > > The script runs continuously and when running with perl 5.24.0 its > memory footprint increases markedly over time. > The memory footprint did not increase with perl 5.22.0.
Confirmed. No leak in 5.22 or current bleadperl. 5.24.0 and 5.24.1-RC1 both leak. The two 5.25 dev releases I have installed are: v5.25.1-75-gcbef69c v5.25.2-196-g620f73f The first leaks. The second does not. So it was fixed somewhere between them. -- Father Chrysostomos
Download (untitled) / with headers
text/plain 769b
On Wed Jul 20 13:10:46 2016, sprout wrote: Show quoted text
> On Wed Jul 20 07:37:14 2016, sc@charnell.plus.com wrote:
> > A perl script I use to parse the XML produced from a Current cost > > energy meter looks to have a memory leak > > after I updated from perl 5.22.0 to perl 5.24.0. > > > > The script runs continuously and when running with perl 5.24.0 its > > memory footprint increases markedly over time. > > The memory footprint did not increase with perl 5.22.0.
> > Confirmed. No leak in 5.22 or current bleadperl. 5.24.0 and 5.24.1- > RC1 both leak. > > The two 5.25 dev releases I have installed are: > > v5.25.1-75-gcbef69c > v5.25.2-196-g620f73f > > The first leaks. The second does not. So it was fixed somewhere > between them.
Thanks. -- Stewart Charnell
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 683b
By eyeballing the memory usage, I have determined that this bug was fixed by the three commits: ee072c898947f5fee316f1381b29ad692addcf05 7eec73eb790f7c4982edfc28c17c011e8a072490 0bf54b1ecaec8f6d80845d6cb77d62f8c9f4c415 and that this ticket is the same as #128313 (which I'm merging it into). That ticket only showed a leak with 'use re "strict"'. This shows it can happen outside that. Since this is a regression, this is a candidate for fixing in a 5.24 maintenance release. I have created a patch which has those 3 commits merged into one (the 2nd and 3rd were modifications of the first) and attached it to this message, and am adding it to the blocker and maint votes lists
Subject: 0001-perl-128313-Fix-leak-in-perl-5.24-with-strict-and-re.patch
From 961c05f10d8a0c851d14092322f3b0afc372b7e0 Mon Sep 17 00:00:00 2001 From: Yves Orton <demerphq@gmail.com> Date: Wed, 8 Jun 2016 18:42:30 +0200 Subject: [PATCH] [perl #128313] Fix leak in perl 5.24 with strict and regex posix char classes This patch is a refinement of one written by Dan Collins. Any thanks for this patch should go to him. --- regcomp.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/regcomp.c b/regcomp.c index 0b1e606..86173db 100644 --- a/regcomp.c +++ b/regcomp.c @@ -199,6 +199,7 @@ struct RExC_state_t { scan_frame *frame_head; scan_frame *frame_last; U32 frame_count; + AV *warn_text; #ifdef ADD_TO_REGEXEC char *starttry; /* -Dr: where regtry was called. */ #define RExC_starttry (pRExC_state->starttry) @@ -290,6 +291,7 @@ struct RExC_state_t { #define RExC_frame_count (pRExC_state->frame_count) #define RExC_strict (pRExC_state->strict) #define RExC_study_started (pRExC_state->study_started) +#define RExC_warn_text (pRExC_state->warn_text) /* Heuristic check on the complexity of the pattern: if TOO_NAUGHTY, we set * a flag to disable back-off on the fixed/floating substrings - if it's @@ -6764,6 +6766,7 @@ Perl_re_op_compile(pTHX_ SV ** const patternp, int pat_count, #endif } + pRExC_state->warn_text = NULL; pRExC_state->code_blocks = NULL; pRExC_state->num_code_blocks = 0; @@ -13764,8 +13767,8 @@ 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(); \ - av_push(warn_text, Perl_newSVpvf(aTHX_ \ + if (! RExC_warn_text ) RExC_warn_text = (AV *) sv_2mortal((SV *) newAV()); \ + av_push(RExC_warn_text, Perl_newSVpvf(aTHX_ \ WARNING_PREFIX \ text \ REPORT_LOCATION, \ @@ -13896,7 +13899,6 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state, bool has_opening_colon = FALSE; int class_number = OOB_NAMEDCLASS; /* Out-of-bounds until find valid class */ - AV* warn_text = NULL; /* any warning messages */ const char * possible_end = NULL; /* used for a 2nd parse pass */ const char* name_start; /* ptr to class name first char */ @@ -13912,6 +13914,9 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state, PERL_ARGS_ASSERT_HANDLE_POSSIBLE_POSIX; + if (posix_warnings && RExC_warn_text) + av_clear(RExC_warn_text); + if (p >= e) { return NOT_MEANT_TO_BE_A_POSIX_CLASS; } @@ -14529,14 +14534,8 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state, ADD_POSIX_WARNING(p, "there is no terminating ']'"); } - 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); - } + if (posix_warnings && RExC_warn_text && av_top_index(RExC_warn_text) > -1) { + *posix_warnings = RExC_warn_text; } } else if (class_number != OOB_NAMEDCLASS) { -- 2.5.0
To: Perl RT Bug Tracker <perlbug-followup [...] perl.org>
Subject: Re: [perl #128680] Suspected memory leak with match operation
Date: Mon, 8 Aug 2016 15:29:54 +0200
From: demerphq <demerphq [...] gmail.com>
I pushed the same patch as commit dfd347fdcdbaf5a157586b05825005dc352b0e83 (origin/yves/backport_128313, yves/backport_128313) Author: Yves Orton <demerphq@gmail.com> Date: Wed Jun 8 18:42:30 2016 +0200 [perl #128313] Fix leak in perl 5.24 with strict and regex posix char classes move warning text to RExC_state (via RExC_warn_text) This way we reuse the same AV each time, and avoid various refcount bookkeeping issues, all at a relatively modest cost (IMO) This patch is the result of detective work and inital patches by Dan Collins with additional coding to take advantage of the regexp engine internals by Yves Orton after feedback from Karl Williams. It is a squash of the following commits in blead: 222c4b0094b4145d06cb164bedd2a66a3141203b [perl #128313] test for memory leak in POSIX classes 0bf54b1ecaec8f6d80845d6cb77d62f8c9f4c415 fixup, guard av_top_index() for null RExC_warn_text 7eec73eb790f7c4982edfc28c17c011e8a072490 move warning text to RExC_state (via RExC_warn_text) ee072c898947f5fee316f1381b29ad692addcf05 [perl #128313] Fix leak in perl 5.24 with strict and regex posix char classes I mention this only because I took some time and care to write a more detailed commit message. :-) Yves On 6 August 2016 at 21:01, Karl Williamson via RT <perlbug-followup@perl.org> wrote: Show quoted text
> By eyeballing the memory usage, I have determined that this bug was fixed by the three commits: > ee072c898947f5fee316f1381b29ad692addcf05 > 7eec73eb790f7c4982edfc28c17c011e8a072490 > 0bf54b1ecaec8f6d80845d6cb77d62f8c9f4c415 > > and that this ticket is the same as #128313 (which I'm merging it into). That ticket only showed a leak with 'use re "strict"'. This shows it can happen outside that. > > Since this is a regression, this is a candidate for fixing in a 5.24 maintenance release. I have created a patch which has those 3 commits merged into one (the 2nd and 3rd were modifications of the first) and attached it to this message, and am adding it to the blocker and maint votes lists > > > --- > via perlbug: queue: perl5 status: open > https://rt.perl.org/Ticket/Display.html?id=128680 > > From 961c05f10d8a0c851d14092322f3b0afc372b7e0 Mon Sep 17 00:00:00 2001 > From: Yves Orton <demerphq@gmail.com> > Date: Wed, 8 Jun 2016 18:42:30 +0200 > Subject: [PATCH] [perl #128313] Fix leak in perl 5.24 with strict and regex > posix char classes > > This patch is a refinement of one written by Dan Collins. > > Any thanks for this patch should go to him. > --- > regcomp.c | 21 ++++++++++----------- > 1 file changed, 10 insertions(+), 11 deletions(-) > > diff --git a/regcomp.c b/regcomp.c > index 0b1e606..86173db 100644 > --- a/regcomp.c > +++ b/regcomp.c > @@ -199,6 +199,7 @@ struct RExC_state_t { > scan_frame *frame_head; > scan_frame *frame_last; > U32 frame_count; > + AV *warn_text; > #ifdef ADD_TO_REGEXEC > char *starttry; /* -Dr: where regtry was called. */ > #define RExC_starttry (pRExC_state->starttry) > @@ -290,6 +291,7 @@ struct RExC_state_t { > #define RExC_frame_count (pRExC_state->frame_count) > #define RExC_strict (pRExC_state->strict) > #define RExC_study_started (pRExC_state->study_started) > +#define RExC_warn_text (pRExC_state->warn_text) > > /* Heuristic check on the complexity of the pattern: if TOO_NAUGHTY, we set > * a flag to disable back-off on the fixed/floating substrings - if it's > @@ -6764,6 +6766,7 @@ Perl_re_op_compile(pTHX_ SV ** const patternp, int pat_count, > #endif > } > > + pRExC_state->warn_text = NULL; > pRExC_state->code_blocks = NULL; > pRExC_state->num_code_blocks = 0; > > @@ -13764,8 +13767,8 @@ 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(); \ > - av_push(warn_text, Perl_newSVpvf(aTHX_ \ > + if (! RExC_warn_text ) RExC_warn_text = (AV *) sv_2mortal((SV *) newAV()); \ > + av_push(RExC_warn_text, Perl_newSVpvf(aTHX_ \ > WARNING_PREFIX \ > text \ > REPORT_LOCATION, \ > @@ -13896,7 +13899,6 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state, > bool has_opening_colon = FALSE; > int class_number = OOB_NAMEDCLASS; /* Out-of-bounds until find > valid class */ > - AV* warn_text = NULL; /* any warning messages */ > const char * possible_end = NULL; /* used for a 2nd parse pass */ > const char* name_start; /* ptr to class name first char */ > > @@ -13912,6 +13914,9 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state, > > PERL_ARGS_ASSERT_HANDLE_POSSIBLE_POSIX; > > + if (posix_warnings && RExC_warn_text) > + av_clear(RExC_warn_text); > + > if (p >= e) { > return NOT_MEANT_TO_BE_A_POSIX_CLASS; > } > @@ -14529,14 +14534,8 @@ S_handle_possible_posix(pTHX_ RExC_state_t *pRExC_state, > ADD_POSIX_WARNING(p, "there is no terminating ']'"); > } > > - 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); > - } > + if (posix_warnings && RExC_warn_text && av_top_index(RExC_warn_text) > -1) { > + *posix_warnings = RExC_warn_text; > } > } > else if (class_number != OOB_NAMEDCLASS) { > -- > 2.5.0 > >
-- perl -Mre=debug -e "/just|another|perl|hacker/"
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 415b
On Mon Aug 08 06:30:43 2016, demerphq wrote: Show quoted text
> I pushed the same patch as > > commit dfd347fdcdbaf5a157586b05825005dc352b0e83 > (origin/yves/backport_128313, yves/backport_128313) > Author: Yves Orton <demerphq@gmail.com> > Date: Wed Jun 8 18:42:30 2016 +0200 > > [perl #128313] Fix leak in perl 5.24 with strict and regex posix > char classes
Does that mean we have three votes now? -- Father Chrysostomos
To: Perl RT Bug Tracker <perlbug-followup [...] perl.org>
Subject: Re: [perl #128313] Memory leak in perl 5.24.0 when use re qw[strict] is used
CC: Perl5 Porteros <perl5-porters [...] perl.org>
From: demerphq <demerphq [...] gmail.com>
Date: Mon, 8 Aug 2016 16:00:57 +0200
Download (untitled) / with headers
text/plain 605b
On 8 August 2016 at 15:37, Father Chrysostomos via RT <perlbug-followup@perl.org> wrote: Show quoted text
> On Mon Aug 08 06:30:43 2016, demerphq wrote:
>> I pushed the same patch as >> >> commit dfd347fdcdbaf5a157586b05825005dc352b0e83 >> (origin/yves/backport_128313, yves/backport_128313) >> Author: Yves Orton <demerphq@gmail.com> >> Date: Wed Jun 8 18:42:30 2016 +0200 >> >> [perl #128313] Fix leak in perl 5.24 with strict and regex posix >> char classes
> > Does that mean we have three votes now?
If I wasnt part of the original vote count then yes. Yves -- perl -Mre=debug -e "/just|another|perl|hacker/"
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 360b
On Mon Aug 08 07:01:23 2016, demerphq wrote: Show quoted text
> On 8 August 2016 at 15:37, Father Chrysostomos via RT > <perlbug-followup@perl.org> wrote:
> > Does that mean we have three votes now?
> > If I wasnt part of the original vote count then yes.
Are you aware of the maint-votes branch? I have added your vote to the votes-5.24.xml file. -- Father Chrysostomos
Download (untitled) / with headers
text/plain 313b
Thank you for filing this report. You have helped make Perl better. With the release today of Perl 5.26.0, this and 210 other issues have been resolved. Perl 5.26.0 may be downloaded via: https://metacpan.org/release/XSAWYERX/perl-5.26.0 If you find that the problem persists, feel free to reopen this ticket.


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org