in reply to Re^5: Net::SSH2 test connection to remote host
in thread Net::SSH2 test connection to remote host

The problem is that eval { } makes a scope for the statements to be executed. On scope exit, code may be executed (for instance, DESTROY methods). That code can clear $@, or set it - it may even call eval itself.

That's the reason you cannot trust $@ to be set if, and only if, the eval failed.

Here's an example. 

#!/usr/bin/perl

use 5.010;

use strict;
use warnings;

use Test::More tests => 3;

eval {my $x = bless [], 'one'}; is $@, "";
eval {my $x = bless [], 'two'}; is $@, "";
eval {my $x = bless [], 'three'; die "foo\n";}; is $@, "foo\n";

__END__
1..3
ok 1
ok 2
ok 3

[download]
Now we change the code by adding some DESTROY methods - and note that this is the only change in the code. 
#!/usr/bin/perl

use 5.010;

use strict;
use warnings;

use Test::More tests => 3;

eval {my $x = bless [], 'one'}; is $@, "";
eval {my $x = bless [], 'two'}; is $@, "";
eval {my $x = bless [], 'three'; die "foo\n";}; is $@, "foo\n";

sub one::DESTROY {1;}
sub two::DESTROY {$@ = "foo";}
sub three::DESTROY {$@ = "";}

__END__
1..3
ok 1
not ok 2
#   Failed test at /tmp/test line 11.
#          got: 'foo'
#     expected: ''
not ok 3
#   Failed test at /tmp/test line 12.
#          got: ''
#     expected: 'foo
# '
# Looks like you failed 2 tests of 3.

[download]
As you can see, after the second eval $@ is set where you don't expect it to be (the eval succeeded), and while the third eval died, $@ is false.

And note that while I did direct assignment to $@ in the DESTROY methods, I could also have used evals in the DESTROY methods to create false positive or negatives.

Replies are listed 'Best First'.
Re^7: Net::SSH2 test connection to remote host
by salva (Canon) on Oct 01, 2008 at 07:36 UTC
    In my opinion, DESTROY methods should not have side effects like changing $@ (or $? or $!), and if they do, it is a bug!

    And I don't see that programming defensively against this kind of bugs is the right thing to do. It just complicates the code unnecessarily. The only reason I see to do that is when the DESTROY method is outside your control (i.e. in a package from CPAN).

    But if it's your own code, just... 

    sub DESTROY {
  local $@;
  ...
}

    [download]
[reply]
[d/l]
      In my opinion, DESTROY methods should not have side effects like changing $@ (or $? or $!), and if they do, it is a bug!

      That's an opinion shared by others, but other people disagree. See for instance a thread about this on p5p this summer. Note that if DESTROY wouldn't propagate changes to $@, $! or $? DESTROY would be different from other functions, because $@, $! and $? behave differently. It also means you can have code that throws an exception, but afterwards $@ isn't set (your local $@ suffers from the same problem): 

          sub one::DESTROY {
        die "foo\n";
    }
    eval {my $x = bless [], 'one'};
    say "\$\@ = $@";
    __END__
    $@ =     (in cleanup) foo

      [download]
      but 
          sub one::DESTROY {
        local $@;
        die "foo\n";
    }
    eval {my $x = bless [], 'one'};
    say "\$\@ = $@";
    __END__
    $@ =

      [download]
      This is far from ideal as well, and I doubt it's much better than the current situation.

      People have suggested a new variable, @@, where new errors get pushed onto, instead of having them override $@. But noone has written a patch to actually do this.

[reply]
[d/l]
[select]
        ok, let me refine my comment:
        DESTROY methods should not have side effects like changing $@ unless they do it while dieing

        In other words, I have no problem if some DESTROY method dies and changes $@ while doing it.

        In practice, what I mean is that inside a DESTROY sub, if eval is used, $@ should be localized for its scope (and the same applies for $! and IO operations and $? and system, fork, etc.)

        For instance, I see no problem with this DESTROY method: 

        sub DESTROY {
  my $self = shift;
  { local $@; eval { $self->whatever() }; }
  die "cleanup failed" unless $self->cleanup;
}

        [download]

        Regarding the p5p discussion, it was about whether this no-side-effects-from-DESTROY thing should be implemented at the interpreter level.

[reply]
[d/l]
[select]

In Section Seekers of Perl Wisdom