http://www.perlmonks.org?node_id=1019527

jbert has asked for the wisdom of the Perl Monks concerning the following question:

Hi.

I've recently had a bunch of "fun" chasing down leaks in code (mine and others) caused by accidental capture of variables in callbacks.

e.g. something like:

sub foo { my $self = shift; $self->do_something(on_success => sub { do_stuff(); $self->log("done stuff"); do_more_stuff(); $self->log("done more stuff"); });

Assuming that do_something hides the callback within $self, we've just created a cycle which if it doesn't get cleared (e.g. on an error path) will cause a leak (could leak mem, could leak other resources such as network connections. badness.)

Note that the logging could have been added casually, and doesn't look evil on a review diff.

One idea being bounced around here would be a pragma which declared "I don't want any vars to be captured implicitly" and a way of marking which vars are ok to capture.

The thinking is that this would make it clear and explicit what vars are being captured, so it is easy to see them and reason about them. Something like:

use NoClose; sub foo { # Still a cycle, but now a more obvious one! $self->do_something(on_success => sub : capture('$self') { do_stuff(); $self->log("done stuff"); do_more_stuff(); $self->log("done more stuff"); });

With the idea being that any vars which a closure would capture would be compile-time errors unless they are referenced in the 'capture' attribute.

So...is this madness? And has it already been implemented? Or do people resolve this pain in a different way?

(I know about weak refs, the issue is more about making it easy to know what to weaken.)

[Edit, added comment to code to emphasise that this doesn't prevent cycles, just makes them more visible.

Replies are listed 'Best First'.
Re: Managing capture
by LanX (Saint) on Feb 19, 2013 at 12:10 UTC
    to answer the question "how do I pass a method as a callback"

    doSomething is evidently a method, so it gets my ($self,%args) =@_ in the first line.

    Now if you wanna call an anonymous method $args{on_succes}, do something like

    $self->$args{on_succes}(ARG_LIST);

    Assure that the callback itself takes self again from the args!

     on_success => sub { my ($self,%args); }

    Cheers Rolf

    UPDATE:

    sigh ... there is a parsing problem with $self->$args{on_succes}(ARG_LIST);

    You'll need to use a temporary var $code_ref

    my $code_ref=$args{on_succes}; $self->$code_ref(ARG_LIST);
Re: Managing capture
by LanX (Saint) on Feb 19, 2013 at 11:58 UTC
    What about a pragma to always start a method with

    my ($self, $arg1,...) = @_;

    ???

    Why did you think that $self is automatically provided?

    > So...is this madness?

    Closures are an essential part of Perl's strength, unlikely anybody will help to cripple the language.

    You might either consider changing to another OO frmework or using snippets in your IDE (like yasnippet in emacs) to facilitate writing methods.

    Cheers Rolf

      > Closures are an essential part of Perl's strength, unlikely anybody will help to cripple the language.

      They are a powerful feature sure. A *pragma* (or cpan module) which allows people to declare their intent more clearly doesn't change the default behaviour for people who don't use it.

      The same argument applies to other pragmas (strict/warnings etc).

Re: Managing capture
by Anonymous Monk on Feb 19, 2013 at 11:45 UTC

      > No substitute for competence and mandatory destructors :)

      Competence is always nice sure, but I don't understand how destructors help. They aren't invoked unless an object is destroyed, which it won't be if it paricipates in a cycle.

        Competence is always nice sure, but I don't understand how destructors help. They aren't invoked unless an object is destroyed, which it won't be if it paricipates in a cycle.

        I think you understand pefectly :)

        If you're accepting callbacks and storing them in an object, you can made a practice of using PadWalker and checking for closures

        A slightly simpler check could be made using Devel::Refcount but why bother when PadWalker can do it :)

        To elaborate on competence, you said Note that the logging could have been added casually, and doesn't look evil on a review diff. except that it should -- when doing reviews you should be checking for cycles, esp with anonymous subs

        But I don't really know what you mean by "review diff"

        Also, you could write a perlcritic policy, it isn't too hard :)

      forget about sanity, the "closure" it deals with is fatal warning, see perllexwarn, it warns about odd cases of closure where closure won't happen , nothing to do with disabling closures intentionally

      Another implementation idea is B::Xref, this shows up as

        Subroutine foo
          Package (lexical)
            $other            i5
            $self             i4, 11
          Package ?
            ??                &11
      
      under subroutine foo, anonymous package, referencing $self

      Could ask the B::Lint folks for an implementation :)

Re: Managing capture
by sundialsvc4 (Abbot) on Feb 19, 2013 at 13:44 UTC

    Hey, let's quit being Anonymous Monk when calling one another “incompetent.”   I happen to think that this is a damm good idea ... how do we implement it?

    To my way of thinking, the computer should be (at least able to be) doing everything that it possibly can to detect errors, as automatically as it can.   Especially the subtle, nasty, disruptive ones like these.   Any sort of tool, with a decorator as described, that would even make a serious dent in this particular issue would be extremely valuable.   To me, it doesn’t matter that an eagle-eye or what have you could find it without assistance:   that’s what computers are for.   Let the hacking begin.

      Hey, let's quit being Anonymous Monk when calling one another “incompetent.”

      I did not call anyone incompetent , learn to read you troll