Re: Rethrowing with die $@ considered harmful

I don't know if its directly related, but this makes me think of a LDAP helper module I wrote for a system I now have in production.

I won't go into too much detail, but (among other things) it had a "login" and "logout" method. Each updated a global session data hash with info about the current user. These methods throw exceptions if something goes wrong. Nothing fancy so far.

Later, I wanted to make a facility for administrators to "become" a normal user (think Unix "su"). However, I wanted to make it so that if the operation failed, it would restore the existing session. So, I wrote this code:

    my %ubackup = %udat;
    eval {
        $self->logout;
        $self->login(username => $username);
    };
    if($@) {
        %udat = %ubackup;
        die $@;
    }
[download]

Now this looks fine, but any errors were being mysteriously swallowed. Turns out the culprit was my LDAP object destructor, which did this:

sub DESTROY {
    my ($self) = @_;

    eval {
        $self->_unbind;
    };
}
[download]

ie shut down the connection, and ignore errors.

The login/logout methods create LDAP objects as part of their operation, so of course they were being destroyed at the end of the method. The call to _unbind was trampling $@.

This was a nightmare to track down. The solution was to simply localise $@ in the destructor.

    local $@;
    eval {
        $self->_unbind;
    };
[download]

Comment on Re: Rethrowing with die $@ considered harmful Select or Download Code

Replies are listed 'Best First'.
Re^2: Rethrowing with die $@ considered harmful by bluto (Curate) on Jul 20, 2006 at 22:49 UTC
This was a nightmare to track down. The solution was to simply localise $@ in the destructor. Yeah, side effects and destructors don't mix well accoding to perlobj... Since DESTROY methods can be called at unpredictable times, it is important that you localise any global variables that the method may update. In particular, localise $@ if you use "eval {}" and localise $? if you use "system" or backticks. You can see where I got bit by this a few years ago here Devious Destructor. It would be nice if perl did this for you (but perhaps it can't?) since sometimes it's not obvious that what you are calling is altering globals, and it's not easy to remember to do this.	[reply]
Re^3: Rethrowing with die $@ considered harmful (best practices) by tye (Sage) on Jul 21, 2006 at 15:27 UTC
So I think the better solution is to extend this warning / practice to FETCH- and STORE-type subs, rather than having code that uses $@ have to be paranoid that $@ might be tied to something that uses eval in its FETCH. Such a FETCH should be considered "broken" from a "best practices" point of view. Being paranoid about $@ changing on you is not a bad practice. I'm just saying that in comparison, using eval w/o local($@) in a FETCH is a worse practice. And that is especially true for a FETCH meant to be used with $@. - tye	[reply]


Keep It Simple, Stupid
	PerlMonks