You cannot call the ->DESTROY method yourself and expect anything sane to happen. It is only meant to be called when the object is *actually* being deallocated. If you do that then you'll need to somehow note in your object that its been "pre-destroyed". Your rebless idea has no bearing on this and is purely obfuscatory. Its no help, abandon it now.

Really though, you need to fix your code so you're not keeping extra references. I'll guess you've got other things leaking memory as well. I'd go through your code with a fine-tooth comb and consider whether you've created any cyclical references - perl's garbage collector won't clean those up for you. ($self->{foo} = $self)

sub DESTROY {
    my $self = shift;

    # Oops! I already destroyed myself
    return if exists $self->{_PRE_DESTROY};
     $self->{_PRE_DESTROY} = 1;


    # Do whatever your cleanup action is.
}
[download]

What the rebless accomplishes though, is that any attempt to interact with the object after the explicit call to DESTROY() will be safely routed to the null class. I agree, your proposed solution would work, but in that case I'd really want to modify all the methods to check _PRE_DESTROY to be safe (I suppose I could do that with a subclass and an AUTOLOAD that performs that check).

I'd really like to just force the ref count of the object to zero and null out all references that exist to it, regardless of where they are. But it seems like the rebless is the next best thing (i.e., I may not be able to find all the references right now, but I can make them all point into nothingness).

And yes, I intend to find the dangling references. This most definitely is a hack. I suspect it's an unintended closure rather than a circular reference.

Thanks for responding.

You can preemptively decrement the refcount to the variable but that just means that the dangling reference will now have a pointer into memory that doesn't belong to it anymore. I guess you could have an EvilHack package which just throws errors when any method in it is called. Maybe you'd find out where its being referenced from that way.

For some creative package names consider Blessing into bitstrings. ;-)</code>

You are wrong. The reblessing does acomplish what mla wants. It will ensure that the DESTROY doesn't do its work agan and it will even ensure no other method does anything after the DESTROY. And if mla changes the warn() to carp it might even help him in finding the places where is the object being used after it should have been destroyed.

I'd do one more change though. I'd move the reblessing into the DESTROY:

package Null;
use strict;
use Carp;

our $AUTOLOAD;

sub AUTOLOAD { carp __PACKAGE__ . " $AUTOLOAD CALLED" }

package Foo;
use strict;
use Carp;

sub new { bless [], __PACKAGE__ }

sub DESTROY {
  my $self = shift;

  carp (__PACKAGE__ . " DESTROY CALLED");
  bless $self, 'Null';
}

package main;

my $s1 = Foo->new;
my $s2 = $s1;
$s1->DESTROY;

$s2->DoSomething();
[download]

You are right though that in the long run mla should go over his code and either get rid of any cyclical references or at least weaken()s them.

Jenda
Always code as if the guy who ends up maintaining your code will be a violent psychopath who knows where you live.
   -- Rick Osborne

Edit by castaway: Closed small tag in signature

Class::DBI uses the technique of blessing an object into an undefined class after deleting an object. It creates problems because class methods that should continue working like <code>dbi_commit<code> for committing the deletion stop working.

• Build your object
• Store it into a pnote: $r->pnote("holder", $my_object);
• Weaken your original reference: use Scalar::Util qw(weaken); weaken($my_object);

-- Randal L. Schwartz, Perl hacker
Be sure to read my standard disclaimer if this is a reply.

I really have to echo iburrell on this issue ... having a DESTROY method that cleans stuff up is fine, but it should never be the only way to clean that stuff up.

In terms of "Good Practices", you shouldn't call DESTROY directly, nor should you depend on DESTROY being called in a timely manner ... but you can take advantage of DESTROY to try and clean up things you might have overlooked.

Why? Why is it better to have a clean_up() method that DESTROY() calls instead of just a DESTROY() method?

It seems like it potentially makes things more fragile, since it implies that clean_up() can occur independently of destruction, which is not the case. That's precisely the concern -- I don't want to release the resource while there are still active objects.

Yes, I could add a flag to disable the object after clean_up, and modify all methods to inspect it, but that doesn't require a separate clean_up() method, so what does it actually accomplish?

Why is it better to have a clean_up() method that DESTROY() calls instead of just a DESTROY() method?

You pretty much answered that question in your orriginal post...

I could call the DESTROY method explicity, which would release the lock. But then, I want to make certain that DESTROY is not called again, implicitly, if/when the object is really reclaimed, since the lock will have already been released, making it unsafe.

DESTROY is "magic", it's whole purpose for existing is to be implicitly called ... eventually. But you never know when, or in what order it will be called on for multiple objects -- And quite frankly, you never know what else perl might want to do it's calling your DESTROY method.

The simple fact that you think you need to explicitly call DESTROY at a specific time is a strong indication to me that you should rename that method. That's not what DESTROY is for.