New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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: