views:

173

answers:

5

I have a Perl project were I just had a problem by making a circular package call. The code below demonstrates the problem.

When this is executed, each package will call the other until all of the memory of the computer is consumed and it locks up. I agree that this is a bad design and that circular calls like this should not be made in the design, but my project is sufficiently big that I would like to detect this at run time.

I have read about the weaken function and Data::Structure::Util, but I have not figured out a way to detect if there is a circular package load (I am assume, because a new copy is being made at each iteration and stored in each copy of the $this hash). Any ideas?

use system::one;

my $one = new system::one(); 

package system::one;

use strict;

use system::two;

sub new {
  my ($class) = @_; 
  my $this = {};  
  bless($this,$class); 
  # attributes
  $this->{two} = new system::two();
  return $this; 
} 

package system::two;

use strict;

use system::one;

sub new {
  my ($class) = @_; 
  my $this = {};  
  bless($this,$class); 
  # attributes
  $this->{one} = new system::one();
  return $this; 
}
+5  A: 

The fact that these are in separate packages has nothing at all to do with the fact that this runs infinitely, consuming all available resources. You're calling two methods from within one another. This isn't circular reference, it's recursion, which is not the same thing. In particular, weaken won't help you at all. You'd get exactly the same effect from:

sub a {
    b();
}

sub b {
    a();
}

a();

The best way to avoid this is don't do that. More usefully, if you have to write recursive functions try not to use multiple functions in the recursion chain, but simply the one, so you have an easier time mentally keeping track of where your calls should terminate.

As to how to detect whether something like this is happening, you would have to do something simple like increment a variable with your recursion depth and terminate (or return) if your depth exceeds a certain value. But you really shouldn't have to rely on that, it's similar to writing a while loop and using an increment there to make sure your function doesn't run out of control. Just don't recurse over a set unless you know how and when it terminates.

Another relevant question would be what are you trying to accomplish in the first place?

Adam Bellaire
+3  A: 

I suggest making a routine called something like break_constructor_recursion() that uses caller() to examine the call stack like so:

Find out what method in what package just called me.

Look up the rest of the call stack seeing if that same method in that same package is anywhere further up.

If so, die() with something appropriate.

Then you add a call to break_constructor_recursion() in your constructors. If the constructor is being called from inside itself, it'll bomb out.

Now, this can throw false positives; it's not impossible for a constructor to be legitimately called inside itself. If you have issues with that, I'd say just have it look for some N additional occurrences of the constructor before it identifies an error. If there are 20 calls to system::two::new() on the stack, the chances that you aren't recursing are pretty low.

chaos
+2  A: 

The classic break on double recursion is to use a state variable to determine if you are already inside a function:

{
    my $in_a;
    sub a {
        return if $in_a; #do nothing if b(), or someone b() calls, calls a()
        $in_a = 1;
        b();
        $in_a = 0;
    }
}

You can do whatever you want if $in_a is true, but dieing or returning is common. If you are using Perl 5.10 or later you can use the state function instead of nesting the function in its own scope:

sub a {
    state $in_a;
    return if $in_a; #do nothing if b(), or someone b() calls, calls a()
    $in_a = 1;
    b();
    $in_a = 0;
}
Chas. Owens
I like this solution to the problem. Don't know why it never occurred to me this was recursion. Thanks!
Dan Littlejohn
+2  A: 

Here, have some code too. :)

sub break_recursion(;$) {
    my $allowed = @_ ? shift : 1;
    my @caller = caller(1);
    my $call = $caller[3];
    my $count = 1;
    for(my $ix = 2; @caller = caller($ix); $ix++) {
        croak "found $count levels of recursion into $call"
            if $caller[3] eq $call && ++$count > $allowed;
    }
}

sub check_recursion(;$) {
    my $allowed = @_ ? shift : 1;
    my @caller = caller(1);
    my $call = $caller[3];
    my $count = 1;
    for(my $ix = 2; @caller = caller($ix); $ix++) {
        return 1
            if $caller[3] eq $call && ++$count > $allowed;
    }
    return 0;
}

These are called like:

break_recursion(); # to die on any recursion
break_recursion(5); # to allow up to 5 levels of recursion
my $recursing = check_recursion(); # to check for any recursion
my $recursing = check_recursion(10); # to check to see if we have more than 10 levels of recursion.

Might CPAN these, I think. If anyone has any thoughts about that, please share.

chaos
I like this solution too. Guess I will play and see if this one or Mr Owen's solution works better for my project.
Dan Littlejohn
Either will probably do fine. The main reason you'd need mine is if there are any cases where some amount of recursion in your constructors is necessary/desirable.
chaos
Oh, hey, and I should note that mine is way more of a performance hit, and is something you'd want to disable in production.
chaos
A: 

use warnings;

without warnings:

#!/usr/bin/perl 

use strict;

sub foo {
    foo(); 
}

foo();

-

$ perl script.pl 
^C # after death 

with warnings:

#!/usr/bin/perl 

use strict;
use warnings;

sub foo {
    foo(); 
}

foo();

-

$ perl script.pl 
Deep recursion on subroutine "main::foo" at script.pl line 7.
^C # after death 

Always always use warnings.

use warnings FATAL => qw( recursion );

#!/usr/bin/perl 

use strict;
use warnings FATAL => qw( recursion );

sub foo {
    foo(); 
}

foo();

-

$ perl script.pl 
Deep recursion on subroutine "main::foo" at script.pl line 7.
$ 
Kent Fredric