perl-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stas Bekman <s...@stason.org>
Subject Re: [mp2 patch] APR::Bucket::alloc_ => APR::BucketAlloc
Date Tue, 28 Dec 2004 00:45:50 GMT
Stas Bekman wrote:
> Joe, please take a look. It was actually a trivial fix in tests, since 
> it's used only in one place. So it probably shouldn't affect any users.
> 
> One problem I have is with:
> 
>   APR::Bucket::alloc_destroy moved to APR::BucketAlloc::DESTROY
> 
> So now one doesn't need to call destroy explicitly since DESTROY will be 
> called automatically. Is that a good thing? I think so. The problem that 
> most automatic DESTROY methods in mp2 are not very good. If someone 
> explicitly calls DESTROY and then perl calls DESTROY, it's a certain 
> segfault. That could be fixed, by a wrapper which eats the heart of the 
> object once DESTROY is called replaced with a hole (similar to 
> APR::Pool::DESTROY, but simpler). Do you think we should bother?
> 
> Of course dropping DESTROY methods and replacing those with plan 
> destroy() will avoid this problem, and if a user calls destroy() more 
> than once, is it their fault that they get a segfault? it certainly is, 
> but it's nice to  not to have perl code segfault no matter what. So I'm 
> in favor of keeping DESTROY, but ensuring that those don't segfault if 
> called more than once on the same object.

One thing I didn't take into an account:

   my $ba   = $r->connection->bucket_alloc;

now will be autodestroyed via DESTROY, which ensures yet another segfault, 
since that $ba must not be destroyed by user code. Of course we may need 
the same thing as we did with APR::Pool to distinguish native from custom 
pools in DESTROY, but I tend to go on the lazy side and just add 
s/DESTROY/destroy/.

Opinions?

Looking at the whole mp2 API, it'd be nice to be consistent. At the moment 
we have:

                DESTROY  destroy
-------------------------------
APR::Brigade:      -        +
APR::Bucket:       -        +
APR::Pool:         +        +
Apache::SubRequest +        -
APR::UUID          +        -
APR::ThreadMutex   +        -
APR::BucketAlloc   ?        ?

So should APR::Brigade and APR::Bucket have DESTROY method added?

If so should destroy() be removed or left as an alias? (probably not a 
good idea, as it'll break people's code).

Should those classes that have only DESTROY have destroy() added as an alias?

-- 
__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Mime
View raw message