| Subject: | The use of local in Catalyst::View::TT->render is unreliable. |
| Date: | Wed, 01 Apr 2009 17:27:30 +1100 |
| To: | bug-Catalyst-View-TT [...] rt.cpan.org |
| From: | Daniel Pittman <daniel [...] rimspace.net> |
G'day.
I was trying to use the documented 'additional_template_paths' feature
of a view derived from Catalyst::View::TT, and the additional paths were
not being used.
After quite some debugging I discovered that this is because the code in
render is broken:
local $self->{include_path} =
[ @{ $vars->{additional_template_paths} }, @{ $self->{include_path} } ]
if ref $vars->{additional_template_paths};
The value '$self->{include_path}' is an ARRAYREF that was passed to the
Template::Toolkit object as the search path. The code above tries to
modify that using 'local' to preserve changes.
Unfortunately, that doesn't actually work: local creates a new ARRAYREF,
containing the alternate values, and restores that — rather than modify
the content of the array and restore it.
This is reproducible with both:
This is perl, v5.10.0 built for x86_64-linux-gnu-thread-multi
This is perl, v5.8.8 built for i486-linux-gnu-thread-multi
Both are from Debian, but I don't imagine that makes a great deal of
difference.
I have attached a little test script that demonstrates this, although
you can easily reproduce the same results yourself.
To remedy this I did a little poking, discovered (as you probably did)
that the interface to modifying the include path for a Template::Toolkit
instance is insanely horrible, and went back to mutating the array.
This code works:
sub render {
my $self = shift; # for calling next...
my $c = $_[0]; # because I need to access it.
my @old;
# This is very, very ugly. Not as ugly as using the API, and at least it
# works, eh, without depending on quirks of the Perl implementation.
if (exists $c->stash->{additional_template_paths}) {
# Preserve the old content.
@old = @{ $self->{include_path} || [] };
# Set the new content without changing the array location.
@{ $self->{include_path} } =
(@{ $c->stash->{additional_template_paths} },
@{ $self->{include_path} });
}
my $rval = $self->NEXT::render(@_);
if (exists $c->stash->{additional_template_paths}) {
# Yay, restore the previous path value.
@{ $self->{include_path} } = @old;
}
return $rval;
}
Clearly, not as nice as your code, but it has the benefit of actually
working robustly.[1]
Regards,
Daniel
Footnotes:
[1] This will fail if we have multiple threads accessing the same
template object instance, but I think other parts of Catalyst would
fail there first, *AND* the existing code should suffer the same
weakness.
If one the Template::Toolkit API wasn't so awful in the area, eh?
Message body is not shown because sender requested not to inline it.