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:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Reply via email to