memory leak when including a file with "use utf8" #13587
Comments
From @lavv17Created by @lavv17Every time perl includes a file with "use utf8" and text constants Here is my test code: for (0..1e5) { x.inc contains: Perl Info |
From @cpansproutOn Fri Feb 07 02:54:49 2014, lavv17f@gmail.com wrote:
Confirmed. It’s not leaking SVs (which would make the leak -- Father Chrysostomos |
|
The RT System itself - Status changed from 'new' to 'open' |
From @cpansproutOn Sun Feb 09 07:04:24 2014, sprout wrote:
perl 5.14.4 in unaffected. -- Father Chrysostomos |
From @jkeenanOn Sun Feb 09 07:05:42 2014, sprout wrote:
But I don't seem to be getting this error on blead: ##### Somewhat less clean results on Darwin/PPC, substituting 'rss=' for 'size=', at: |
From perl5-porters@perl.orgJames Keenan wrote:
I can still reproduce it on blead. It was caused by this commit: commit e8b3448 Cleanup of pad fetching and storing.... Presumably the return value of bytes_from_utf8 needs to be freed. |
From @HugmeirOn Mon, Feb 10, 2014 at 1:16 AM, Father Chrysostomos <sprout@cpan.org>wrote:
Thanks for tracking this one down, Father C! I've been trying to fix this My main problem right now is how to add a test for this. Would there be any my $null = File::Spec->devnull; That makes the test run on my darwin box, but just skips it on my phone, |
From @unobevalgrind reports a small memory leak when 'use utf8;' is used in perl 5.20.0, the latest patch release candidate for perl 5.20 (5.20.2-RC1), and the latest development release (5.21.8). The leak is greater when using all optional features (-E). The attached file, leak.pl, can be used to test different versions of perl. I used something similar to leak.pl on blead at the end of last week and no leak was reported. |
From @tonycozOn Tue Feb 10 10:04:58 2015, tiro wrote:
By default perl won't release memory on exit. This is to speed up tear-down of the perl process, since the operating system is going to release that memory anyway when the process exits. There's a few ways to force the release: - if you have a debugging perl, set PERL_DESTRUCT_LEVEL=2 $ PERL_DESTRUCT_LEVEL=2 valgrind -q --leak-check=yes ~/perl/5.20.1-dbg/bin/perl -e 'use strict' - creating a thread: $ valgrind -q --leak-check=yes ~/perl/5.20.1-dbg/bin/perl -e 'use strict; use threads; threads->create(sub{})->join' - the Perl::Destruct::Level module on CPAN. Tony |
|
The RT System itself - Status changed from 'new' to 'open' |
From @tonycozOn Tue Feb 10 15:23:31 2015, tonyc wrote:
Just loading threads is enough, I missed an assignment. Tony |
From @unobeTony, I attempted to simply reply to your e-mail but didn't see the reply reflected Thank you for looking at this and for the quick reply. The test script I previously provided had perl hardcoded so probably wasn't I didn't have a copy of perl with threads support, so built perl v5.20.1 from $ git checkout v5.20.1 You are in 'detached HEAD' state. You can look around, make experimental If you want to create a new branch to retain commits you create, you may git checkout -b new_branch_name HEAD is now at 4f732e9... perldelta - Finalize acknowledgements $ ./Configure -Dusedevel -Dusethreads -Doptimize=-O2 -Dloclibpth='/usr/lib64 /usr/local/lib64' -esd $ make $ ./perl -v This is perl 5, version 20, subversion 1 (v5.20.1) built for x86_64-linux-thread-multi $ valgrind -q --leak-check=yes ./perl -Ilib -Mthreads -e '$_' $ valgrind -q --leak-check=yes ./perl -Ilib -Mthreads -Mutf8 -e '1' $ valgrind -q --leak-check=yes ./perl -Ilib -Mthreads -Mutf8 -e 'use strict' $ valgrind -q --leak-check=yes ./perl -Ilib -Mthreads -Mutf8 -e '$_' $ valgrind -q --leak-check=yes ./perl -Ilib -Mthreads -Mutf8 -e '$_ab' $ valgrind -q --leak-check=yes ./perl -Ilib -Mthreads -Mutf8 -e 'sub {}' $ valgrind -q --leak-check=yes ./perl -Ilib -Mthreads -Mutf8 -e 'sub a {}' $ valgrind -q --leak-check=yes ./perl -Ilib -Mthreads -Mutf8 -e 'sub ab {}' Tony Cook via RT wrote on Tue, Feb 10, 2015 at 03:32:42PM PST:
|
From @unobeTony, Thank you for looking at this and for the quick reply. The test script I previously provided had perl hardcoded so probably wasn't I didn't have a copy of perl with threads support, so built perl v5.20.1 from $ git checkout v5.20.1 You are in 'detached HEAD' state. You can look around, make experimental If you want to create a new branch to retain commits you create, you may git checkout -b new_branch_name HEAD is now at 4f732e9... perldelta - Finalize acknowledgements $ ./Configure -Dusedevel -Dusethreads -Doptimize=-O2 -Dloclibpth='/usr/lib64 /usr/local/lib64' -esd $ make $ ./perl -v This is perl 5, version 20, subversion 1 (v5.20.1) built for x86_64-linux-thread-multi $ valgrind -q --leak-check=yes ./perl -Ilib -Mthreads -e '$_' $ valgrind -q --leak-check=yes ./perl -Ilib -Mthreads -Mutf8 -e '1' $ valgrind -q --leak-check=yes ./perl -Ilib -Mthreads -Mutf8 -e 'use strict' $ valgrind -q --leak-check=yes ./perl -Ilib -Mthreads -Mutf8 -e '$_' $ valgrind -q --leak-check=yes ./perl -Ilib -Mthreads -Mutf8 -e '$_ab' $ valgrind -q --leak-check=yes ./perl -Ilib -Mthreads -Mutf8 -e 'sub {}' $ valgrind -q --leak-check=yes ./perl -Ilib -Mthreads -Mutf8 -e 'sub a {}' $ valgrind -q --leak-check=yes ./perl -Ilib -Mthreads -Mutf8 -e 'sub ab {}' Tony Cook via RT wrote on Tue, Feb 10, 2015 at 03:32:42PM PST:
|
From @tonycozOn Thu Feb 12 08:06:08 2015, tiro wrote:
No, it's a real bug, still present in maint-5.20. The leak isn't from loading utf8.pm itself, but from doing variable name lookups in its context, so with one variable, one block is leaked: tony@mars:.../git/perl$ valgrind -q --leak-check=full ./perl -Ilib -Mthreads -Mutf8 -e '$x' with 3 variables, 3 blocks are leaked: tony@mars:.../git/perl$ valgrind -q --leak-check=full ./perl -Ilib -Mthreads -Mutf8 -e '$x,$y,$z' This isn't an issue in blead, since the leaking code is no longer present - all pad names are in utf-8 so there's no need to convert. The attached patch appears to fix the problem for maint-5.20. Tony |
From @maukeAm 25.02.2015 um 01:22 schrieb Tony Cook via RT:
What patch? I see no attachment. -- |
From @tonycozOn Wed Feb 25 00:01:55 2015, plokinom@gmail.com wrote:
Oops, here it is. Tony |
From @tonycoz0001-perl-123786-don-t-leak-the-temp-utf8-copy-of-namepv.patchFrom c701019ebd89baf31d6b03f5200e3f8955bfaa92 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Wed, 25 Feb 2015 11:20:55 +1100
Subject: [perl #123786] don't leak the temp utf8 copy of namepv
---
pad.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/pad.c b/pad.c
index fed2892..8d008fd 100644
--- a/pad.c
+++ b/pad.c
@@ -961,6 +961,7 @@ Perl_pad_findmy_pvn(pTHX_ const char *namepv, STRLEN namelen, U32 flags)
I32 offset;
const AV *nameav;
SV **name_svp;
+ bool free_pv = false;
PERL_ARGS_ASSERT_PAD_FINDMY_PVN;
@@ -973,6 +974,7 @@ Perl_pad_findmy_pvn(pTHX_ const char *namepv, STRLEN namelen, U32 flags)
if (flags & padadd_UTF8_NAME) {
bool is_utf8 = TRUE;
namepv = (const char*)bytes_from_utf8((U8*)namepv, &namelen, &is_utf8);
+ free_pv = true;
if (is_utf8)
flags |= padadd_UTF8_NAME;
@@ -982,6 +984,8 @@ Perl_pad_findmy_pvn(pTHX_ const char *namepv, STRLEN namelen, U32 flags)
offset = pad_findlex(namepv, namelen, flags,
PL_compcv, PL_cop_seqmax, 1, NULL, &out_sv, &out_flags);
+ if (free_pv)
+ Safefree(namepv);
if ((PADOFFSET)offset != NOT_IN_PAD)
return offset;
--
1.7.10.4
|
From @jkeenanOn Tue Feb 24 16:22:20 2015, tonyc wrote:
Tony: I checked out tag v5.20.1 and confirmed the leaks reported with valgrind. I then applied your patch, rebuilt (sh ./Configure -des -Dusedevel -Dusethreads -Doptimize=-O2 on Ubuntu 14.04) and confirmed that the leaks had been eliminated. However, when I checked out blead (commit e11fa37), built with threads as above, and then ran the valgrind tests, memory leaks were still being reported. 2 examples: ##### Based on your remarks, I would have expected blead to be leak-free in this respect. Am I overlooking something? (New to valgrind.) Thank you very much. -- |
From @tonycozOn Wed, Feb 25, 2015 at 03:48:38PM -0800, James E Keenan via RT wrote:
...
That's a different leak, introduced with the OP_MULTIDEREF blead no longer includes the conversion code that produces the leak in Tony |
From @tonycozOn Mon, Apr 20, 2015 at 02:13:23PM +0200, Dirk Stöcker wrote:
Discuss this on the p5p mailing list please. If you include: [perl #123786] in the subject it will be included in the ticket. Tony |
From @tonycozOn Mon Apr 20 06:16:04 2015, tonyc wrote:
The attached patch should fix it. The interface of bytes_from_utf8() seems to be designed to leak :( Tony |
From @tonycoz0001-perl-123786-don-t-leak-the-temp-utf8-copy-of-namepv.patchFrom 9b3706b6b71c09adc56e84dd4e67666e505a33b7 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Tue, 21 Apr 2015 16:13:13 +1000
Subject: [PATCH] [perl #123786] don't leak the temp utf8 copy of namepv
---
pad.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/pad.c b/pad.c
index fed2892..f22c3c5 100644
--- a/pad.c
+++ b/pad.c
@@ -976,8 +976,10 @@ Perl_pad_findmy_pvn(pTHX_ const char *namepv, STRLEN namelen, U32 flags)
if (is_utf8)
flags |= padadd_UTF8_NAME;
- else
+ else {
flags &= ~padadd_UTF8_NAME;
+ SAVEFREEPV(namepv);
+ }
}
offset = pad_findlex(namepv, namelen, flags,
--
1.7.10.4
|
From dixoncx@gmail.comHi, This leak is present in perl 5.16 as well. -- |
From @tonycozOn Fri Jun 12 07:01:20 2015, dixoncx@gmail.com wrote:
5.16 is no longer supported by perl5-porters. From a quick look at pad.c the patch should apply against maint-5.16. Tony |
From @steve-m-hayOn Mon Apr 20 23:19:30 2015, tonyc wrote:
Thanks. This fix has now been applied to the maint-5.20 branch in commit 8d89c05 and will shortly be released in perl-5.20.3. |
|
@steve-m-hay - Status changed from 'open' to 'pending release' |
From @ppisarOn 2015-08-19, Steve Hay via RT <perlbug-followup@perl.org> wrote:
Is this bug duplicate of bug #121200 (memory leak when including a file with -- Petr |
From @tonycozOn Wed Aug 26 04:57:08 2015, ppisar wrote:
Certainly looks like it, the test code in 121200 no longer leaks in maint-5.20. I'll merge the tickets. Tony |
|
@tonycoz - Status changed from 'open' to 'pending release' |
From @khwilliamsonThank you for submitting this report. You have helped make Perl better. Perl 5.24.0 may be downloaded via https://metacpan.org/release/RJBS/perl-5.24.0 |
|
@khwilliamson - Status changed from 'pending release' to 'resolved' |
Migrated from rt.perl.org#121200 (status was 'resolved')
Searchable as RT121200$
The text was updated successfully, but these errors were encountered: