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

Owner: Nobody
Requestors: zdm [at] softvisio.net
Cc:
AdminCc:

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



Subject: Memory leak in concatenation operator overload
To: perlbug [...] perl.org
From: "zdm [...] softvisio.net" <zdm [...] softvisio.net>
Date: Tue, 22 Jan 2019 12:49:53 +0200
Download (untitled) / with headers
text/plain 4.5k
This is a bug report for perl from zdm@softvisio.net, generated with the help of perlbug 1.41 running under perl 5.28.1. ----------------------------------------------------------------- [Please describe your issue here] New memory leak was introduces in perl 5.28. Please, see the discussion here: https://stackoverflow.com/questions/54304341/perl-overload-concatenation-operator-problem #!/usr/bin/env perl use strict; use warnings; use Devel::Refcount qw[refcount]; package AAA { use Devel::Refcount qw[refcount]; use overload '.' => sub { print 'CONCAT, REFCOUNT: ', refcount( $_[0] ), "\n"; # return AAA->new; return $_[0]; }, fallback => 0; sub new { return bless {}, $_[0] } sub DESTROY { print "DESTROY\n" } } print "--- start\n"; { my $o = AAA->new; my $s = '1' . ( '2' . ( '3' . ( '4' . ( '5' . $o ) ) ) ); print "--- exit scope\n"; print 'REFCOUNT: ', refcount($o), "\n"; } print "--- end\n"; 1; Output: --- start CONCAT, REFCOUNT: 1 CONCAT, REFCOUNT: 3 CONCAT, REFCOUNT: 5 CONCAT, REFCOUNT: 7 CONCAT, REFCOUNT: 9 --- exit scope REFCOUNT: 6 --- end DESTROY Refs count should stay untouched, but it increased on each iteration. [Please do not change anything below this line] ----------------------------------------------------------------- --- Flags: category=core severity=high --- Site configuration information for perl 5.28.1: Configured by mockbuild at Mon Jan 21 14:05:45 UTC 2019. Summary of my perl5 (revision 5 version 28 subversion 1) configuration: Platform: osname=linux osvers=4.16.13-300.fc28.x86_64 archname=x86_64-linux-ld uname='linux 0b8553fb90eb4b24a5147b6ec2b37883 4.16.13-300.fc28.x86_64 #1 smp wed may 30 14:31:00 utc 2018 x86_64 x86_64 x86_64 gnulinux ' config_args='-des -Dusemorebits -Duselargefiles -Dprefix=/usr/perlbrew/perls/perl-5.28.1 -Duserelocatableinc -Dman1dir=none -Dman3dir=none' hint=recommended useposix=true d_sigaction=define useithreads=undef usemultiplicity=undef use64bitint=define use64bitall=define uselongdouble=define usemymalloc=n default_inc_excludes_dot=define bincompat5005=undef Compiler: cc='cc' ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64' optimize='-O2' cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include' ccversion='' gccversion='8.2.1 20181215 (Red Hat 8.2.1-6)' gccosandvers='' intsize=4 longsize=8 ptrsize=8 doublesize=8 byteorder=12345678 doublekind=3 d_longlong=define longlongsize=8 d_longdbl=define longdblsize=16 longdblkind=3 ivtype='long' ivsize=8 nvtype='long double' nvsize=16 Off_t='off_t' lseeksize=8 alignbytes=16 prototype=define Linker and Libraries: ld='cc' ldflags =' -fstack-protector-strong -L/usr/local/lib' libpth=/usr/local/lib /usr/lib /lib/../lib64 /usr/lib/../lib64 /lib /lib64 /usr/lib64 /usr/local/lib64 libs=-lpthread -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat perllibs=-lpthread -ldl -lm -lcrypt -lutil -lc libc=libc-2.28.so so=so useshrplib=false libperl=libperl.a gnulibc_version='2.28' Dynamic Linking: dlsrc=dl_dlopen.xs dlext=so d_dlsymun=undef ccdlflags='-Wl,-E' cccdlflags='-fPIC' lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector-strong' --- @INC for perl 5.28.1: /mnt/hgfs/projects/pcore-lib/pcore/lib /usr/perlbrew/perls/perl-5.28.1/lib/site_perl/5.28.1/x86_64-linux-ld /usr/perlbrew/perls/perl-5.28.1/lib/site_perl/5.28.1 /usr/perlbrew/perls/perl-5.28.1/lib/5.28.1/x86_64-linux-ld /usr/perlbrew/perls/perl-5.28.1/lib/5.28.1 --- Environment for perl 5.28.1: HOME=/root LANG=en_US.UTF-8 LANGUAGE=en_US.UTF-8 LC_ALL=en_US.UTF-8 LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/usr/perlbrew/bin:/usr/perlbrew/perls/perl-5.28.1/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/mnt/hgfs/projects/pcore-lib/pcore/bin PERL5LIB=/mnt/hgfs/projects/pcore-lib/pcore/lib PERLBREW=command perlbrew PERLBREW_HOME=/usr/perlbrew PERLBREW_MANPATH=/usr/perlbrew/perls/perl-5.28.1/man PERLBREW_PATH=/usr/perlbrew/bin:/usr/perlbrew/perls/perl-5.28.1/bin PERLBREW_PERL=perl-5.28.1 PERLBREW_ROOT=/usr/perlbrew PERLBREW_SHELLRC_VERSION=0.86 PERLBREW_VERSION=0.86 PERL_BADLANG (unset) PERL_CPANM_HOME=/tmp/.cpanm PERL_CPANM_OPT=--metacpan --from https://cpan.metacpan.org/ SHELL=/bin/bash
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 2.4k
On Tue, 22 Jan 2019 02:50:40 -0800, zdm@softvisio.net wrote: Show quoted text
> New memory leak was introduces in perl 5.28. > Please, see the discussion here: > https://stackoverflow.com/questions/54304341/perl-overload- > concatenation-operator-problem > > #!/usr/bin/env perl > > use strict; > use warnings; > use Devel::Refcount qw[refcount]; > > package AAA { > use Devel::Refcount qw[refcount]; > > use overload > '.' => sub { > print 'CONCAT, REFCOUNT: ', refcount( $_[0] ), "\n"; > > # return AAA->new; > return $_[0]; > }, > fallback => 0; > > sub new { return bless {}, $_[0] } > > sub DESTROY { print "DESTROY\n" } > } > > print "--- start\n"; > > { > my $o = AAA->new; > > my $s = '1' . ( '2' . ( '3' . ( '4' . ( '5' . $o ) ) ) ); > > print "--- exit scope\n"; > print 'REFCOUNT: ', refcount($o), "\n"; > } > > print "--- end\n"; > > 1; > > Output: > > --- start > CONCAT, REFCOUNT: 1 > CONCAT, REFCOUNT: 3 > CONCAT, REFCOUNT: 5 > CONCAT, REFCOUNT: 7 > CONCAT, REFCOUNT: 9 > --- exit scope > REFCOUNT: 6 > --- end > DESTROY > > Refs count should stay untouched, but it increased on each iteration.
This is present in blead too. There's two places the reference count is being incremented. The first is in leave_adjust_stacks() as the return from the '.' overload is being processed: (gdb) bt #0 S_SvREFCNT_inc (sv=0x555555d204a8) at inline.h:194 #1 0x0000555555760634 in Perl_leave_adjust_stacks (from_sp=0x555555d4ae10, to_sp=0x555555d4ae08, gimme=2 '\002', pass=0) at pp_hot.c:4793 #2 0x0000555555760a48 in Perl_pp_leavesub () at pp_hot.c:4902 #3 0x00005555556f4b41 in Perl_runops_debug () at dump.c:2536 #4 0x0000555555607358 in Perl_amagic_call (left=0x555555d39c40, right=0x555555d43580, method=70, flags=0) at gv.c:3525 #5 0x0000555555748b19 in Perl_pp_multiconcat () at pp_hot.c:1060 Each of these is added to the tmps stack. The second is in Perl_pp_multiconcat at the end of the magical value fallback code: (gdb) bt #0 S_SvREFCNT_inc (sv=0x555555d204a8) at inline.h:194 #1 0x000055555578ab26 in Perl_sv_setsv_flags (dstr=0x555555d43580, sstr=0x555555d39880, flags=1538) at sv.c:4261 #2 0x0000555555748d85 in Perl_pp_multiconcat () at pp_hot.c:1106 if (is_append) SvTAINT(targ); else { sv_setsv(targ, left); <--- SvSETMAGIC(targ); } targ here comes from the PAD: SV **svp = &(PAD_SVl(PL_op->op_targ)); targ = *svp; Tony
From: Dave Mitchell <davem [...] iabyn.com>
To: Tony Cook via RT <perlbug-followup [...] perl.org>
CC: perl5-porters [...] perl.org
Date: Thu, 24 Jan 2019 12:00:41 +0000
Subject: Re: [perl #133789] Memory leak in concatenation operator overload
Download (untitled) / with headers
text/plain 1.2k
On Tue, Jan 22, 2019 at 04:17:47PM -0800, Tony Cook via RT wrote: Show quoted text
> > Refs count should stay untouched, but it increased on each iteration.
> > This is present in blead too. > > There's two places the reference count is being incremented. >
... Show quoted text
> The second is in Perl_pp_multiconcat at the end of the magical value fallback code: > > (gdb) bt > #0 S_SvREFCNT_inc (sv=0x555555d204a8) at inline.h:194 > #1 0x000055555578ab26 in Perl_sv_setsv_flags (dstr=0x555555d43580, > sstr=0x555555d39880, flags=1538) at sv.c:4261 > #2 0x0000555555748d85 in Perl_pp_multiconcat () at pp_hot.c:1106 > > if (is_append) > SvTAINT(targ); > else { > sv_setsv(targ, left); <--- > SvSETMAGIC(targ); > } > > targ here comes from the PAD: > > SV **svp = &(PAD_SVl(PL_op->op_targ)); > targ = *svp;
After each call to multiconcat that returns an overloaded value, its targ will end up holding an extra ref to the overloaded value. So its not a open-ended leak in the sense that calling multiconcat in a loop won't leak more and more objects, but just delay freeing the last object. I'll need to think a bit about the best way to avoid assigning to the targ. -- Diplomacy is telling someone to go to hell in such a way that they'll look forward to the trip
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 1.8k
On Tue, 22 Jan 2019 10:50:40 GMT, zdm@softvisio.net wrote: Show quoted text
> This is a bug report for perl from zdm@softvisio.net, > generated with the help of perlbug 1.41 running under perl 5.28.1. > > > ----------------------------------------------------------------- > [Please describe your issue here] > > New memory leak was introduces in perl 5.28. > Please, see the discussion here: > https://stackoverflow.com/questions/54304341/perl-overload- > concatenation-operator-problem > > #!/usr/bin/env perl > > use strict; > use warnings; > use Devel::Refcount qw[refcount]; > > package AAA { > use Devel::Refcount qw[refcount]; > > use overload > '.' => sub { > print 'CONCAT, REFCOUNT: ', refcount( $_[0] ), "\n"; > > # return AAA->new; > return $_[0]; > }, > fallback => 0; > > sub new { return bless {}, $_[0] } > > sub DESTROY { print "DESTROY\n" } > } > > print "--- start\n"; > > { > my $o = AAA->new; > > my $s = '1' . ( '2' . ( '3' . ( '4' . ( '5' . $o ) ) ) ); > > print "--- exit scope\n"; > print 'REFCOUNT: ', refcount($o), "\n"; > } > > print "--- end\n"; > > 1; > > Output: > > --- start > CONCAT, REFCOUNT: 1 > CONCAT, REFCOUNT: 3 > CONCAT, REFCOUNT: 5 > CONCAT, REFCOUNT: 7 > CONCAT, REFCOUNT: 9 > --- exit scope > REFCOUNT: 6 > --- end > DESTROY > > Refs count should stay untouched, but it increased on each iteration. >
FWIW, bisection points to this commit: ##### e839e6ed99c6b25aee589f56bb58de2f8fa00f41 is the first bad commit commit e839e6ed99c6b25aee589f56bb58de2f8fa00f41 Author: David Mitchell <davem@iabyn.com> Date: Tue Aug 8 18:42:14 2017 +0100 Add OP_MULTICONCAT op Allow multiple OP_CONCAT, OP_CONST ops, plus optionally an OP_SASSIGN or OP_STRINGIFY, to be combined into a single OP_MULTICONCAT op, which can make things a *lot* faster: 4x or more. ... ##### Thank you very much. -- James E Keenan (jkeenan@cpan.org)
To: Tony Cook via RT <perlbug-followup [...] perl.org>
From: Dave Mitchell <davem [...] iabyn.com>
Subject: Re: [perl #133789] Memory leak in concatenation operator overload
Date: Tue, 5 Feb 2019 14:15:14 +0000
CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 3.1k
On Thu, Jan 24, 2019 at 12:00:41PM +0000, Dave Mitchell wrote: Show quoted text
> On Tue, Jan 22, 2019 at 04:17:47PM -0800, Tony Cook via RT wrote:
> > > Refs count should stay untouched, but it increased on each iteration.
> > > > This is present in blead too. > > > > There's two places the reference count is being incremented. > >
> ... >
> > The second is in Perl_pp_multiconcat at the end of the magical value fallback code: > > > > (gdb) bt > > #0 S_SvREFCNT_inc (sv=0x555555d204a8) at inline.h:194 > > #1 0x000055555578ab26 in Perl_sv_setsv_flags (dstr=0x555555d43580, > > sstr=0x555555d39880, flags=1538) at sv.c:4261 > > #2 0x0000555555748d85 in Perl_pp_multiconcat () at pp_hot.c:1106 > > > > if (is_append) > > SvTAINT(targ); > > else { > > sv_setsv(targ, left); <--- > > SvSETMAGIC(targ); > > } > > > > targ here comes from the PAD: > > > > SV **svp = &(PAD_SVl(PL_op->op_targ)); > > targ = *svp;
> > After each call to multiconcat that returns an overloaded value, its targ > will end up holding an extra ref to the overloaded value. So its not a > open-ended leak in the sense that calling multiconcat in a loop won't leak > more and more objects, but just delay freeing the last object. > > I'll need to think a bit about the best way to avoid assigning to the > targ.
Now fixed in blead with this commit: commit 4e521aaf3ed717774455b3906bd5aa46bc397319 Author: David Mitchell <davem@iabyn.com> AuthorDate: Tue Feb 5 13:48:21 2019 +0000 Avoid leak in multiconcat with overloading. RT #133789 In the path taken through pp_multiconcat() when one or more args have side-effects such tieing or overloading, multiconcat has to decide whether to just return the result of all the concatting as-is, or to first assign it to an expression or variable if the op includes an implicit assign (such as $lex = x.y.z or $a[0] = x.y.z). The code was getting this right for those two cases, and was also getting it right for the append cases ($lex .= x.y.z and $a[0] .= x.y.z), which don't need assigns. But for the bare case (x.y.z) it was assigning to the op's targ as well as returning the value. Hence leaking a reference until destruction of the sub and its pad. This commit stops the assign in that last case. which is part of this just-pushed branch: Author: David Mitchell <davem@iabyn.com> AuthorDate: Tue Feb 5 14:04:32 2019 +0000 [MERGE] various overload fixups This branch contains several commits which simplify the code concerning the processing of a value returned by an overload method, and specifically whether that value should be returned as-is by the op, or assigned to the targ / stack value: $lex = x op y) and (x op= y) respectively. The final commit fixes a bug in pp_multiconcat. That op bypasses most of the code in those earlier commits and "rolls it's own", and which was getting the set/assign decision wrong in some cases, causing a leak. -- "Foul and greedy Dwarf - you have eaten the last candle." -- "Hordes of the Things", BBC Radio.
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.30.0, this and 160 other issues have been resolved. Perl 5.30.0 may be downloaded via: https://metacpan.org/release/XSAWYERX/perl-5.30.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