Do not pass perl globals as arguments to subroutines

Recently I faced this problem when loose an exception message in Mojolicious

What do you think is wrong here

$c->helpers->reply->exception($@) unless eval { $next->(); 1 };

For first sight nothing. We eval code and then catch exception if it is and pass error into sub. But we forget that when arguments are passed into subroutine they are aliased into @_ NOT COPIED (Maybe we forgot that because of it is not documented here ) but, thanks, documented here

 The array @_ is a local array, but its elements are aliases for the actual scalar parameters.

So @_ contain aliases. What does that matter? Look please at this small example:

sub test {
  $@ =  'Oops';
  my( $x ) =  @_;
  print $x; # Oops
}
$@ =  'Exception';
test( $@ );

As you can see between subroutine call and arguments coping there is something may happen and change your global variable. As result you do not get the value you pass into subroutine.

As workaround this problem, I think, you should never pass globals. Take care and pass copies of them:

test( my $copy = $@ );

Aliasing hurts not only perl internal global variables that hurts every global variable:

test( my $copy = $YourPackage::variable );

Hope this small note will save your time when you wanna pass global variable into subroutine.

7 Comments

I would say that if you do your usual first subroutine line:

sub foo {
my ($arg1, $arg2) = @_;
...
}

you will be safe.
But my preferred solution is even better:

use feature 'signatures';

Please, SawyerX, make them default asap :D

Of course this will not save your ass when using third party modules, I know :)

The same kinds of things apply to capture variables.

And if you are writing the subroutine, you might want to avoid returning things like this as well, and for similar reasons.

And if you are writing the subroutine, you might want to avoid returning things like this as well, and for similar reasons.

Returning a value makes a copy, so it doesn’t suffer from this problem (outside of an :lvalue subroutine anyway – which is a rare edge case).

If you are returning a reference, then you may have a problem with the referent being shared with something you did not expect (like DBI’s fetch always returning the same arrayref). But in that case you need a deep copy, not just the shallow copy from copying the ref.

Well, maybe this is cargo cult. Back in 2010 I spent quite a bit of time tracking down a problem where I was returning the value of $+[0] from a subroutine, and the caller was not receiving the value that the subroutine returned. The fix was to force the copy by assigning the value to a lexical variable.

I was never able to figure out exactly what was going on (this was in PPIx-Regexp, it was seen when Perl-Critic called it and I was never able to duplicate the problem outside that rather complex environment, and I encountered it under Perl 5.13.0), but the experience was painful enough that I have made it a practice ever since not to return magic variables, just in case something similar sneaks into a production release of Perl.

I was never able to figure out exactly what was going on […but…] I have made it a practice ever since

That is pretty much cargo cult by definition. 😊 Not that I can fault you for reacting the way you did; so might I.

The fact that a match-related variable was involved makes me suspicious immediately – all of them have various scoping and lifetime oddities for reasons of efficiency and/or DWIM. But who knows if that’s really the issue.

Problem is, this is a situation like with folk remedies: with no tested model of what was going on, your reasoning about why your fix solved your problem is just a guess, so generalising the fix may or may not be protecting you from anything – you don’t really know.

But the experience of tracking down really difficult bugs can be like dealing with a form of gaslighting, and can leave PTSD-like scars, so irrational defences against that are in some sense perfectly rational.

Leave a comment

About KES

user-pic I blog about Perl.