I believe that what you have there will destroy $req when the execute sub exits. At that time, $req will go out of lexical scope, and the local $SIG{INT} will also go out of scope, so there will be no leftover references to $req and it will go away.

If you hadn't made $SIG{INT} local, $req would stay in scope until it was set to something else.

Here's a test program I wrote to confirm this.

#!/usr/bin/perl -w

use strict;

package TestObject;

sub new
{
  my $class = shift;
  my $self = {};
  bless $self,$class;
}

sub somesub
{
  my $self = shift;
  print "Called somesub\n";
}

sub DESTROY
{
  my $self = shift;
  print "DESTROY\n";
}

package main;

print "Entering execute\n";
&execute;
print "Execute completed\n";

sub execute
{
  my $req = TestObject->new
    or die "Couldn't create TestObject\n";
  print "Created TestObject\n";
  local $SIG{INT} = sub { $req->somesub };
  print "Set \$SIG{INT}\n";
  sleep(1);
  print "Done with execute\n";
}
[download]

It produces:

Entering execute
Created TestObject
Set $SIG{INT}
Done with execute
DESTROY
Execute completed
[download]

Update: Fix typo in shebang line.

Trying this on 5.8.0 appears to work. I didn't see any warnings in the documentation on %SIG or local() regarding this.

sub f {
    local $SIG{INT} = sub { die "a" };
    {redo}
}
$\ = "\n";
print $SIG{INT} ? 1 : 0;
eval { f(); };
print $SIG{INT} ? 1 : 0;
[download]

I'm worried that by using a closure like this $req will never get destroyed due to dangling references.

Unless there's something you're not telling us, you should be okay. The operative line is here:

By scoping $SIG{INT} dynamically, you ensure that it will pass out of scope at the end of the block. At that time, the coderef it stored will be thrown away, and assuming that's the only reference to the anonymous sub in question, the anonymous sub's reference count should drop to zero, and it will be harvested; consequently, it will drop its reference to $req->handleInterrupt so that that also can be cleaned up, unless something somewhere else is holding another reference.

Generally speaking, you need to be careful about memory leaks with closures under the same circumstances as with any other references -- primarily when you have circular constructs, e.g., like the following:

sub createcircle {
   my ($foo, $bar) = @_;
   my $qux;
   my $quux = sub {
      if (shift==$foo) { $bar } else { $qux->() }
   };
   $qux = sub { $quux->(shift - 1) };
   return $quux;
}
[download]

$;=sub{$/};@;=map{my($a,$b)=($_,$;);$;=sub{$a.$b->()}}
split//,".rekcah lreP rehtona tsuJ";$\=$ ;->();print$/
[download]

What you describe should work, but I know that there was a bug in some versions of Perl (sorry, I couldn't seem to track it down, nor do I have them to test, but I am pretty sure that Perl 5.005_03 was one of them) where closures like that would never get garbage collected because of a Perl bug.

I would suggest trying the above with a $req with a DESTROY method that tells you when it goes away. See if it is a problem. If it is, then there isn't much you can do about it other than clean up $req yourself and hope that the memory leak doesn't grow too fast.

Thanks all - the target platform is perl 5.6+, so I guess the bug tilly mentioned won't be an issue. I've got this working the way I want now (well - the way I want for an alpha relase :-)) and I've decided on the following:

sub execute {
    my $self = shift;

    my $req = BigHairyDatabaseInteraction->new(...);
    {
        local $SIG{INT} = sub {$req->{_signal} = 2;
                               $req->handleInterrupt; };
        local $SIG{TERM} = sub {$req->{_signal} = 15;
                                $req->handleInterrupt; };
        $req->executeRequest();
    }
    # If a TERM/INT signal has been delivered during the request
    # then pass it on to the program itself:
    if($req->{_signal}) {
        kill $req->{_signal}, $$;
    }
    ... rest of $sth->execute()
[download]

The idea is that I want to override any signal handlers that the calling program may have installed so that I can interrupt the processing on the server as needed, but I also want to make sure that the caller does eventually receive the signal.

So far this appears to work really well...

Michael

One tip that may make sense.

Before you do the local, capture the existing handlers in lexical variables and pass them in to $req's handleInterrupt method.

That way if a particular request type wants to, it can rethrow an exception properly up the chain, rather than just ignoring the poor programmer who is trying to put a signal handler around your code because they really don't want to exit on that signal.

Doing this makes the signal handler system act a little bit more like a proper exception handling system. Not that much more like, mind you, but somewhat more like it.

I _think_ the memory leak you mean only comes up with recursive closures:

my $foo;
$foo=sub { $foo->(@_[1..$#_]) if @_ };
$foo->();
[download]

But like you I dont remember the details...



---
demerphq



First they ignore you, then they laugh at you, then they fight you, then you win.
-- Gandhi

Recursive closures leak memory. That is documented and the fix is to use WeakRef.

What I am talking about is an actual bug, and I darned well know that it was there in 5.005_03 because I encountered it and had to fix a bunch of code that was leaking memory too fast.