Bleadperl b2a2a901 breaks MooseX::Role::Parameterized 0.19 #10697

p5pRT · 2010-10-06T06:52:18Z

Migrated from rt.perl.org#78244 (status was 'rejected')

Searchable as RT78244$

p5pRT · 2010-10-06T06:52:20Z

From @andk

The commit

b2a2a90
Author: David Mitchell <davem@iabyn.com>
Date: Mon Oct 4 15:18:44 2010 +0100

stop map,grep leaking temps [perl #48004]

breaks the tests of SARTAK/MooseX-Role-Parameterized-0.19.tar.gz with a
bunch of error diagnostics, starting with

# Failed test 'threw Regexp ((?^:'Moose::Meta::Role::__ANON__::SERIAL::\d+' requires the method 'alpha' to be implemented by 'Class::MOP::Class::__ANON__::SERIAL::\d+'))'
# at t/006-requires.t line 36.
# expecting: Regexp ((?^:'Moose::Meta::Role::__ANON__::SERIAL::\d+' requires the method 'alpha' to be implemented by 'Class::MOP::Class::__ANON__::SERIAL::\d+'))
# found: Can't locate Moose/Meta/Role/__ANON__/SERIAL/1.pm in @INC (@INC contains: /home/sand/.cpan/build/MooseX-Role-Parameterized-0.19-j0ejK2/inc /home/sand/.cpan/build/MooseX-Role-Parameterized-0.19-j0ejK2/blib/lib /home/sand/.cpan/build/MooseX-Role-Parameterized-0.19-j0ejK2/blib/arch /home/src/perl/repoperls/installed-perls/perl/v5.13.5-284-gb2a2a90/lib/site_perl/5.13.5/x86_64-linux-thread-multi-ld /home/src/perl/repoperls/installed-perls/perl/v5.13.5-284-gb2a2a90/lib/site_perl/5.13.5 /home/src/perl/repoperls/installed-perls/perl/v5.13.5-284-gb2a2a90/lib/5.13.5/x86_64-linux-thread-multi-ld /home/src/perl/repoperls/installed-perls/perl/v5.13.5-284-gb2a2a90/lib/5.13.5 .). at /home/src/perl/repoperls/installed-perls/perl/v5.13.5-284-gb2a2a90/lib/site_perl/5.13.5/x86_64-linux-thread-multi-ld/Class/MOP.pm line 131
# Class::MOP::load_first_existing_class(** Incomplete caller override detected; @DB::args were not set **) called at /home/src/perl/repoperls/installed-perls/perl/v5.13.5-284-gb2a2a90/lib/site_perl/5.13.5/x86_64-linux-thread-multi-ld/Class/MOP.pm line 136
# Class::MOP::load_class(** Incomplete caller override detected; @DB::args were not set **) called at /home/src/perl/repoperls/installed-perls/perl/v5.13.5-284-gb2a2a90/lib/site_perl/5.13.5/x86_64-linux-thread-multi-ld/Moose/Util.pm line 112
# Moose::Util::_apply_all_roles(** Incomplete caller override detected; @DB::args were not set **) called at /home/src/perl/repoperls/installed-perls/perl/v5.13.5-284-gb2a2a90/lib/site_perl/5.13.5/x86_64-linux-thread-multi-ld/Moose/Util.pm line 90
# Moose::Util::apply_all_roles(** Incomplete caller override detected; @DB::args were not set **) called at /home/src/perl/repoperls/installed-perls/perl/v5.13.5-284-gb2a2a90/lib/site_perl/5.13.5/x86_64-linux-thread-multi-ld/Moose/Meta/Class.pm line 87
# Moose::Meta::Class::create(** Incomplete caller override detected; @DB::args were not set **) called at /home/src/perl/repoperls/installed-perls/perl/v5.13.5-284-gb2a2a90/lib/site_perl/5.13.5/x86_64-linux-thread-multi-ld/Class/MOP/Class.pm line 448
# Class::MOP::Class::create_anon_class(** Incomplete caller override detected; @DB::args were not set **) called at /home/src/perl/repoperls/installed-perls/perl/v5.13.5-284-gb2a2a90/lib/site_perl/5.13.5/x86_64-linux-thread-multi-ld/Moose/Meta/Class.pm line 108
# Moose::Meta::Class::create_anon_class(** Incomplete caller override detected; @DB::args were not set **) called at t/006-requires.t line 33
# Test::Exception::throws_ok(** Incomplete caller override detected; @DB::args were not set **) called at t/006-requires.t line 36
[...]
Test Summary Report
-------------------
t/006-requires.t (Wstat: 1280 Tests: 5 Failed: 5)
Failed tests: 1-5
Non-zero exit status: 5
t/007-excludes.t (Wstat: 768 Tests: 3 Failed: 3)
Failed tests: 1-3
Non-zero exit status: 3

the -V of the perl that has this result:

Summary of my perl5 (revision 5 version 13 subversion 5) configuration:
Commit id: b2a2a90
Platform:
osname=linux, osvers=2.6.32-2-amd64, archname=x86_64-linux-thread-multi-ld
uname='linux k81 2.6.32-2-amd64 #1 smp fri feb 12 00:01:47 utc 2010 x86_64 gnulinux '
config_args='-Dprefix=/home/src/perl/repoperls/installed-perls/perl/v5.13.5-284-gb2a2a90 -Dinstallusrbinperl=n -Uversiononly -Dusedevel -des -Ui_db -Duseithreads -Duselongdouble'
hint=recommended, useposix=true, d_sigaction=define
useithreads=define, usemultiplicity=define
useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
use64bitint=define, use64bitall=define, uselongdouble=define
usemymalloc=n, bincompat5005=undef
Compiler:
cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
optimize='-O2',
cppflags='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
ccversion='', gccversion='4.4.4', gccosandvers=''
intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
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 -L/usr/local/lib'
libpth=/usr/local/lib /lib /usr/lib /lib64 /usr/lib64
libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread -lc -lgdbm_compat
perllibs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
libc=/lib/libc-2.11.2.so, so=so, useshrplib=false, libperl=libperl.a
gnulibc_version='2.11.2'
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'

Characteristics of this binary (from libperl):
Compile-time options: MULTIPLICITY PERL_DONT_CREATE_GVSV
PERL_IMPLICIT_CONTEXT PERL_MALLOC_WRAP PERL_USE_DEVEL
USE_64_BIT_ALL USE_64_BIT_INT USE_ITHREADS
USE_LARGE_FILES USE_LONG_DOUBLE USE_PERLIO
USE_PERL_ATOF USE_REENTRANT_API
Built under linux
Compiled at Oct 6 2010 06:30:43
@INC:
/home/src/perl/repoperls/installed-perls/perl/v5.13.5-284-gb2a2a90/lib/site_perl/5.13.5/x86_64-linux-thread-multi-ld
/home/src/perl/repoperls/installed-perls/perl/v5.13.5-284-gb2a2a90/lib/site_perl/5.13.5
/home/src/perl/repoperls/installed-perls/perl/v5.13.5-284-gb2a2a90/lib/5.13.5/x86_64-linux-thread-multi-ld
/home/src/perl/repoperls/installed-perls/perl/v5.13.5-284-gb2a2a90/lib/5.13.5
.

--
andreas

p5pRT · 2010-10-26T15:54:48Z

From @iabyn

On Tue, Oct 05, 2010 at 11:52:20PM -0700, Andreas J. Koenig via RT wrote:

b2a2a90
Author: David Mitchell <davem@iabyn.com>
Date: Mon Oct 4 15:18:44 2010 +0100

stop map,grep leaking temps [perl #48004]

breaks the tests of SARTAK/MooseX-Role-Parameterized-0.19.tar.gz with a
bunch of error diagnostics, starting with

I *think* this is due to a bug (or rather, unreasonable expectations)
in that distribution's test suite. In particular, it has code roughly
like:

sub foo {
map { SomeClass->new->some_method($_) } @_
}

i.e. it creates a temporary object each time through the map block, then
discards it. The change in blead means that the temporary object is freed
at the end of each map iteration, rather than at the next statement
boundary following the map. This means that the object's destructor is
called earlier, which makes the tests fail.

The only question really is whether this is an acceptable change in blead.
I personally think it is. Opinions, anyone?

--
The Enterprise is captured by a vastly superior alien intelligence which
does not put them on trial.
-- Things That Never Happen in "Star Trek" #10

p5pRT · 2010-10-26T15:54:49Z

The RT System itself - Status changed from 'new' to 'open'

p5pRT · 2010-10-26T16:06:51Z

From @tsee

Dave Mitchell wrote:

I *think* this is due to a bug (or rather, unreasonable expectations)
in that distribution's test suite. In particular, it has code roughly
like:
sub foo \{
map \{ SomeClass\->new\->some\_method$$\_$ \} @&#8203;\_
\}
i.e. it creates a temporary object each time through the map block, then
discards it. The change in blead means that the temporary object is freed
at the end of each map iteration, rather than at the next statement
boundary following the map. This means that the object's destructor is
called earlier, which makes the tests fail.

The only question really is whether this is an acceptable change in blead.
I personally think it is. Opinions, anyone?

+1.

I have to admit I had to read your argument slowly. But after things
fell into place, the answer is clearly: It was a bug* before, now it
isn't. We want this change.

Best regards,
Steffen

* bug == "bug in perl if you assume that things get cleaned up 'soon' as
reference counting seems to imply at a superficial level".

p5pRT · 2010-10-26T23:14:26Z

From @ikegami

On Tue, Oct 26, 2010 at 11:54 AM, Dave Mitchell <davem@iabyn.com> wrote:

it has code roughly like:

sub foo {
map { SomeClass->new->some_method($_) } @_
}

i.e. it creates a temporary object each time through the map block, then
discards it. The change in blead means that the temporary object is freed
at the end of each map iteration, rather than at the next statement
boundary following the map.

One shouldn't be surprised when Perl destroys an object as soon as it can.
So the only question is whether this a significant backwards compatibility
issue.

map traditionally returned a list even in void context, so one could expect
the object to survive long enough for map to return them. That said, anyone
who chose to rely on that behaviour would know they are treading on
dangerous ground.

The catch is that there is always a possibility that some relied on that
behaviour unknowingly. It seems to me it would have to be in some pretty
unusual code, and thus unlikely to happen, and likely to happen in code
that's known the be fragile if it does happen.

I don't think this is our problem.

- Eric Brine

p5pRT · 2010-10-27T09:09:45Z

From @druud62

On 2010-10-27 01:14, Eric Brine wrote:

On Tue, Oct 26, 2010 at 11:54 AM, Dave Mitchell<davem@iabyn.com> wrote:

it has code roughly like:
sub foo \{
    map \{ SomeClass\->new\->some\_method$$\_$ \} @&#8203;\_
\}
i.e. it creates a temporary object each time through the map block, then
discards it. The change in blead means that the temporary object is freed
at the end of each map iteration, rather than at the next statement
boundary following the map.
One shouldn't be surprised when Perl destroys an object as soon as it can.
So the only question is whether this a significant backwards compatibility
issue.

map traditionally returned a list even in void context, so one could expect
the object to survive long enough for map to return them. That said, anyone
who chose to rely on that behaviour would know they are treading on
dangerous ground.

The catch is that there is always a possibility that some relied on that
behaviour unknowingly. It seems to me it would have to be in some pretty
unusual code, and thus unlikely to happen, and likely to happen in code
that's known the be fragile if it does happen.

I don't think this is our problem.

There could be map-called-by-map issues popping up, like before when
(IIRC) the inner map didn't destroy until the outer map was done.

Yes, "not our problem" feels appropriate here.

--
Ruud

p5pRT · 2010-10-27T10:46:36Z

@iabyn - Status changed from 'open' to 'rejected'

p5pRT · 2010-11-01T13:47:18Z

From @rurban

2010/10/26 Dave Mitchell <davem@iabyn.com>:

On Tue, Oct 05, 2010 at 11:52:20PM -0700, Andreas J. Koenig via RT wrote:

b2a2a90
Author: David Mitchell <davem@iabyn.com>
Date: Mon Oct 4 15:18:44 2010 +0100

stop map,grep leaking temps [perl #48004]

breaks the tests of SARTAK/MooseX-Role-Parameterized-0.19.tar.gz with a
bunch of error diagnostics, starting with

I *think* this is due to a bug (or rather, unreasonable expectations)
in that distribution's test suite. In particular, it has code roughly
like:

sub foo {
map { SomeClass->new->some_method($_) } @_
}

i.e. it creates a temporary object each time through the map block, then
discards it. The change in blead means that the temporary object is freed
at the end of each map iteration, rather than at the next statement
boundary following the map. This means that the object's destructor is
called earlier, which makes the tests fail.

The only question really is whether this is an acceptable change in blead.
I personally think it is. Opinions, anyone?

We never guaranteed when destruction happens.
But I dislike the early temporary object destruction as it looks like much
slower inside map, to afterwards. Do we really need that?

--
Reini Urban

p5pRT · 2010-11-01T14:04:25Z

From @iabyn

On Mon, Nov 01, 2010 at 02:46:54PM +0100, Reini Urban wrote:

We never guaranteed when destruction happens.
But I dislike the early temporary object destruction as it looks like much
slower inside map, to afterwards.

I don't understand what you mean by slower.

Do we really need that?

Should the following create and keep 10_000 temporary objects and delete
them all at the end, or should it delete one object at the end of each
iteration?

@result = map { Class->new->foo($_} } 1..10_000

Most people would regard the former as leaking behaviour, hence the
change.

--
Indomitable in retreat, invincible in advance, insufferable in victory
-- Churchill on Montgomery

p5pRT · 2010-11-02T09:37:08Z

From @timbunce

On Mon, Nov 01, 2010 at 02:04:01PM +0000, Dave Mitchell wrote:

On Mon, Nov 01, 2010 at 02:46:54PM +0100, Reini Urban wrote:

We never guaranteed when destruction happens.
But I dislike the early temporary object destruction as it looks like much
slower inside map, to afterwards.

I don't understand what you mean by slower.

Do we really need that?

Should the following create and keep 10_000 temporary objects and delete
them all at the end, or should it delete one object at the end of each
iteration?
@&#8203;result = map \{ Class\->new\->foo\($\_\} \} 1\.\.10\_000
Most people would regard the former as leaking behaviour, hence the
change.

Not just leaking, but slower, due to all the allocation overheads
(and, in simple cases, poorer cpu data cache usage).

Tim.

p5pRT · 2010-11-03T15:51:17Z

From @rurban

2010/11/2 Tim Bunce <Tim.Bunce@pobox.com>:

On Mon, Nov 01, 2010 at 02:04:01PM +0000, Dave Mitchell wrote:

On Mon, Nov 01, 2010 at 02:46:54PM +0100, Reini Urban wrote:

We never guaranteed when destruction happens.
But I dislike the early temporary object destruction as it looks like much
slower inside map, to afterwards.

I don't understand what you mean by slower.

Do we really need that?

Should the following create and keep 10_000 temporary objects and delete
them all at the end, or should it delete one object at the end of each
iteration?

@result = map { Class->new->foo($_} } 1..10_000

Most people would regard the former as leaking behaviour, hence the
change.

Not just leaking, but slower, due to all the allocation overheads
(and, in simple cases, poorer cpu data cache usage).

I see, we are recycling it. I was just thinking loud.
--
Reini

p5pRT · 2010-11-03T15:56:07Z

From sartak@gmail.com

2010/10/26 Dave Mitchell <davem@iabyn.com>:

On Tue, Oct 05, 2010 at 11:52:20PM -0700, Andreas J. Koenig via RT wrote:

b2a2a90
Author: David Mitchell <davem@iabyn.com>
Date: Mon Oct 4 15:18:44 2010 +0100

stop map,grep leaking temps [perl #48004]

breaks the tests of SARTAK/MooseX-Role-Parameterized-0.19.tar.gz with a
bunch of error diagnostics, starting with

I *think* this is due to a bug (or rather, unreasonable expectations)
in that distribution's test suite. In particular, it has code roughly
like:

sub foo {
map { SomeClass->new->some_method($_) } @_
}

i.e. it creates a temporary object each time through the map block, then
discards it. The change in blead means that the temporary object is freed
at the end of each map iteration, rather than at the next statement
boundary following the map. This means that the object's destructor is
called earlier, which makes the tests fail.

The only question really is whether this is an acceptable change in blead.
I personally think it is. Opinions, anyone?

As author of the module and tests that broke, I agree that this is an
acceptable change to blead. I'm not sure what I was thinking when I
wrote it, because the new behavior is what I *expect* to happen. In
any case, I've uploaded an 0.20 which changes the code to look like:

my @keepalive;
sub foo {
map {
my $obj = SomeClass->new;
push @keepalive, $obj
$obj->some_method($_)
} @_
}

Works for me!

Thanks,
Shawn

p5pRT closed this as completed Oct 27, 2010

p5pRT added Not a Bug Severity Low distro-Linux leak/refcount/malloc sendToCPAN labels Oct 18, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bleadperl b2a2a901 breaks MooseX::Role::Parameterized 0.19 #10697

Bleadperl b2a2a901 breaks MooseX::Role::Parameterized 0.19 #10697

p5pRT commented Oct 6, 2010

p5pRT commented Oct 6, 2010

p5pRT commented Oct 26, 2010

p5pRT commented Oct 26, 2010

p5pRT commented Oct 26, 2010

p5pRT commented Oct 26, 2010

p5pRT commented Oct 27, 2010

p5pRT commented Oct 27, 2010

p5pRT commented Nov 1, 2010

p5pRT commented Nov 1, 2010

p5pRT commented Nov 2, 2010

p5pRT commented Nov 3, 2010

p5pRT commented Nov 3, 2010

Bleadperl b2a2a901 breaks MooseX::Role::Parameterized 0.19 #10697

Bleadperl b2a2a901 breaks MooseX::Role::Parameterized 0.19 #10697

Comments

p5pRT commented Oct 6, 2010

p5pRT commented Oct 6, 2010

From @andk

p5pRT commented Oct 26, 2010

From @iabyn

p5pRT commented Oct 26, 2010

p5pRT commented Oct 26, 2010

From @tsee

p5pRT commented Oct 26, 2010

From @ikegami

p5pRT commented Oct 27, 2010

From @druud62

p5pRT commented Oct 27, 2010

p5pRT commented Nov 1, 2010

From @rurban

p5pRT commented Nov 1, 2010

From @iabyn

p5pRT commented Nov 2, 2010

From @timbunce

p5pRT commented Nov 3, 2010

From @rurban

p5pRT commented Nov 3, 2010

From sartak@gmail.com