Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve handling of Perl's block eval errors #768

Merged
merged 1 commit into from May 27, 2020
Merged

Improve handling of Perl's block eval errors #768

merged 1 commit into from May 27, 2020

Conversation

knl
Copy link
Contributor

@knl knl commented May 26, 2020

Taken from Perl::Critic:

A common idiom in perl for dealing with possible errors is to use eval
followed by a check of $@/$EVAL_ERROR:

eval {
    ...
};
if ($EVAL_ERROR) {
    ...
}

There's a problem with this: the value of $EVAL_ERROR ($@) can change
between the end of the eval and the if statement. The issue are object
destructors:

package Foo;

...

sub DESTROY {
    ...
    eval { ... };
    ...
}

package main;

eval {
    my $foo = Foo->new();
    ...
};
if ($EVAL_ERROR) {
    ...
}

Assuming there are no other references to $foo created, when the
eval block in main is exited, Foo::DESTROY() will be invoked,
regardless of whether the eval finished normally or not. If the eval
in main fails, but the eval in Foo::DESTROY() succeeds, then
$EVAL_ERROR will be empty by the time that the if is executed.
Additional issues arise if you depend upon the exact contents of
$EVAL_ERROR and both evals fail, because the messages from both will
be concatenated.

Even if there isn't an eval directly in the DESTROY() method code,
it may invoke code that does use eval or otherwise affects
$EVAL_ERROR.

The solution is to ensure that, upon normal exit, an eval returns a
true value and to test that value:

# Constructors are no problem.
my $object = eval { Class->new() };

# To cover the possiblity that an operation may correctly return a
# false value, end the block with "1":
if ( eval { something(); 1 } ) {
    ...
}

eval {
    ...
    1;
}
    or do {
        # Error handling here
    };

Unfortunately, you can't use the defined function to test the result;
eval returns an empty string on failure.

Various modules have been written to take some of the pain out of
properly localizing and checking $@/$EVAL_ERROR. For example:

use Try::Tiny;
try {
    ...
} catch {
    # Error handling here;
    # The exception is in $_/$ARG, not $@/$EVAL_ERROR.
};  # Note semicolon.

"But we don't use DESTROY() anywhere in our code!" you say. That may be
the case, but do any of the third-party modules you use have them? What
about any you may use in the future or updated versions of the ones you
already use?

Taken from `Perl::Critic`:

A common idiom in perl for dealing with possible errors is to use `eval`
followed by a check of `$@`/`$EVAL_ERROR`:

    eval {
        ...
    };
    if ($EVAL_ERROR) {
        ...
    }

There's a problem with this: the value of `$EVAL_ERROR` (`$@`) can change
between the end of the `eval` and the `if` statement. The issue are object
destructors:

    package Foo;

    ...

    sub DESTROY {
        ...
        eval { ... };
        ...
    }

    package main;

    eval {
        my $foo = Foo->new();
        ...
    };
    if ($EVAL_ERROR) {
        ...
    }

Assuming there are no other references to `$foo` created, when the
`eval` block in `main` is exited, `Foo::DESTROY()` will be invoked,
regardless of whether the `eval` finished normally or not. If the `eval`
in `main` fails, but the `eval` in `Foo::DESTROY()` succeeds, then
`$EVAL_ERROR` will be empty by the time that the `if` is executed.
Additional issues arise if you depend upon the exact contents of
`$EVAL_ERROR` and both `eval`s fail, because the messages from both will
be concatenated.

Even if there isn't an `eval` directly in the `DESTROY()` method code,
it may invoke code that does use `eval` or otherwise affects
`$EVAL_ERROR`.

The solution is to ensure that, upon normal exit, an `eval` returns a
true value and to test that value:

    # Constructors are no problem.
    my $object = eval { Class->new() };

    # To cover the possiblity that an operation may correctly return a
    # false value, end the block with "1":
    if ( eval { something(); 1 } ) {
        ...
    }

    eval {
        ...
        1;
    }
        or do {
            # Error handling here
        };

Unfortunately, you can't use the `defined` function to test the result;
`eval` returns an empty string on failure.

Various modules have been written to take some of the pain out of
properly localizing and checking `$@`/`$EVAL_ERROR`. For example:

    use Try::Tiny;
    try {
        ...
    } catch {
        # Error handling here;
        # The exception is in $_/$ARG, not $@/$EVAL_ERROR.
    };  # Note semicolon.

"But we don't use DESTROY() anywhere in our code!" you say. That may be
the case, but do any of the third-party modules you use have them? What
about any you may use in the future or updated versions of the ones you
already use?
@edolstra edolstra merged commit a1e08d8 into NixOS:master May 27, 2020
@edolstra
Copy link
Member

Thanks, much cleaner!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants