Bug #43904 for Moose: Forbid recursive runtime role application

Fri Mar 06 12:45:23 2009 ovid [...] cpan.org - Ticket created

Subject:

Forbid recursive runtime role application

I *think* this should be a bug as I can't find a use case for it and it's bitten me pretty hard. For any role, do this: MyRole->meta->apply($foo) while 1; Eventually you will get fun errors such as: Deep recursion on subroutine "MRO::Compat::__get_linear_isa_dfs" at /home/poec01/trunk/Pips3/deps/lib/perl5/MRO/Compat.pm line 123. Deep recursion on subroutine "Class::MOP::Class::class_precedence_list" at /home/poec01/trunk/Pips3/deps/lib/perl5/i86pc-solaris-thread-multi/Class/MOP/Class.pm line 613. Deep recursion on subroutine "MRO::Compat::__get_linear_isa_dfs" at /home/poec01/trunk/Pips3/deps/lib/perl5/MRO/Compat.pm line 123. Deep recursion on subroutine "MRO::Compat::__get_linear_isa_dfs" at /home/poec01/trunk/Pips3/deps/lib/perl5/MRO/Compat.pm line 123. Deep recursion on subroutine "MRO::Compat::__get_linear_isa_dfs" at /home/poec01/trunk/Pips3/deps/lib/perl5/MRO/Compat.pm line 123. Deep recursion on subroutine "MRO::Compat::__get_linear_isa_dfs" at /home/poec01/trunk/Pips3/deps/lib/perl5/MRO/Compat.pm line 123. Deep recursion on subroutine "Class::MOP::Class::class_precedence_list" at /home/poec01/trunk/Pips3/deps/lib/perl5/i86pc-solaris-thread-multi/Class/MOP/Class.pm line 613. Deep recursion on subroutine "MRO::Compat::__get_linear_isa_dfs" at /home/poec01/trunk/Pips3/deps/lib/perl5/MRO/Compat.pm line 123. Recursive inheritance detected while looking for method '()' in package 'Foo' at /home/poec01/trunk/Pips3/deps/lib/perl5/i86pc-solaris-thread-multi/Class/MOP/Instance.pm line 157. Recursive inheritance detected while looking for method '()' in package 'Foo'. Reapplying a role at runtime to an instance should probably be fatal (I think). This came up as we have test code which fetches a singleton (yeah, I know) and applies a role to that. Cheers, Ovid

Mon Mar 09 17:26:24 2009 autarch [...] urth.org - Correspondence added

Subject:	Re: [rt.cpan.org #43904] Forbid recursive runtime role application
Date:	Mon, 9 Mar 2009 16:25:47 -0500 (CDT)
To:	Curtis 'Ovid' Poe via RT <bug-Moose [...] rt.cpan.org>
From:	Dave Rolsky <autarch [...] urth.org>

On Fri, 6 Mar 2009, Curtis 'Ovid' Poe via RT wrote: Show quoted text

> Reapplying a role at runtime to an instance should probably be fatal (I > think).

Shouldn't it just ignore it? I made a test that applied the same role twice, and there wasn't any deep recursion warning. It seems that doing anything() while 1; is going to be a problem, right? Can you come up with a failing test case? -dave /*============================================================ http://VegGuide.org http://blog.urth.org Your guide to all that's veg House Absolute(ly Pointless) ============================================================*/

Mon Mar 09 17:26:25 2009 The RT System itself - Status changed from 'new' to 'open'

Tue Mar 10 03:35:03 2009 curtis_ovid_poe [...] yahoo.com - Correspondence added

Subject:	Re: [rt.cpan.org #43904] Forbid recursive runtime role application
Date:	Tue, 10 Mar 2009 00:34:48 -0700 (PDT)
To:	bug-Moose [...] rt.cpan.org
From:	Ovid <curtis_ovid_poe [...] yahoo.com>

Show quoted text

----- Original Message ----

> From: "autarch@urth.org via RT" <bug-Moose@rt.cpan.org>

> On Fri, 6 Mar 2009, Curtis 'Ovid' Poe via RT wrote: >

> > Reapplying a role at runtime to an instance should probably be fatal (I > > think).

> > Shouldn't it just ignore it?

Possibly it should just ignore it if it's applying the role directly on top of another application of the same role. That would have stopped my error. Otherwise, if you apply Serializeable::YAML, later apply Serializable::XML and then your code decides it wants Serialiable::YAML again and reapplies that, if you ignore the role application because it's already been applied, then you have the ordering problem that mixins and MI have (last/first win respectively). If you make the re-application fatal, then you forbid tricks like the YAML->XML->YAML swap (which might be a good thing).

> I made a test that applied the same role twice, and there wasn't any deep > recursion warning. It seems that doing

Right. It wouldn't do that.

> anything() while 1; > > is going to be a problem, right? > > Can you come up with a failing test case?

Do you mean "write some tests"? Tough to do since I'm unsure of what behavior is needed here. If you mean a use case, I have one written up at http://use.perl.org/~Ovid/journal/38618 and explained above. Otherwise, if you're just looking for code to demonstrate the error, the following self-contained script does that: { package MyRole; use Moose::Role; } { package Foo; use Moose; with 'MyRole'; } my $foo = Foo->new; my $count = 0; while (1) { print $count++, $/; MyRole->meta->apply($foo); } That throws the warnings on my box after 98 applications. I don't see any perfect solutions here, but one which I think can help things tremendously would be for Moose to detect this problem before Perl does and give clear warnings to help the programmer pin this down. I think the programmer will understand why the repeated application is bad and, more imporantly, I wouldn't have spent hours puzzling over the cryptic error messages before finding this bug :) Cheers, Ovid

Sat Mar 14 14:23:25 2009 stevan.little [...] gmail.com - Correspondence added

Ovid, I don't see this as a bug, but more as an incorrect usage of the feature. You simply need to change your test case to check to see if the role has already been applied. For further justification can be found here -> http://use.perl.org/~Ovid/journal/38618 - Stevan

Sat Mar 14 14:23:26 2009 stevan.little [...] gmail.com - Status changed from 'open' to 'resolved'

Sat Mar 14 14:23:27 2009 stevan.little [...] gmail.com - Given to STEVAN