Re: [mp2] modperl sv bucket has a few problems

Stas Bekman <stas@xxxxxxxxxx> writes:

> There are two problems with modperl sv buckets. You use modperl sv
> buckets when you say:
> 
>    APR::Bucket->new($data);
> 
> 1) If you change $data after creating the bucket, the data in the
> bucket is modified as well. This could lead to unexpected behavior and
> bugs, but with proper documentation it should probably be OK. A better
> solution is to turn the sv into READONLY.
>
> 2) temporary svs cause buckets corruption.
> 
> If you create a real variable, and pass it to APR::Bucket->new() it'll
> get its refcount incremented to ensure that it won't be destroyed by
> Perl. The problem is when new() gets the same SV to create different
> buckets. This is the case with temporary SVs, Perl will re-use the
> same sv again and again when it needs a temp. As the following code
> demonstrates calling APR::Bucket->new(lc $data) passes a temp SV to
> new() and it'll store the same sv address for different buckets. so
> the data of the last bucket will corrupt the data of all the previous
> buckets, since they share the PV slot. Observe for youself:

I haven't looked at the bucket implementation, but FWIW this is
exactly how transient buckets behave in apache's filter stack
(mod_ssl generates these in the input filter chain).  You might
want to implement a setaside callback for sv buckets that would 
copy the SvPVX into a pool-allocated string when called (and presumably 
turn the bucket into a normal pool bucket at that point?).

If you had a setaside, then instead of (or in addition to?)
marking the sv READONLY it might be possible to add setmagic to the sv
that invokes the new setaside callback. That way the sv and the old
bucket might be decoupled when the sv is reassigned, and perhaps
at that point the READONLY flag could be dropped.  

It would certainly require some planning efforts to coordinate
everything,  but I believe it's possible to do something like this.

-- 
Joe Schaefer