perl-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Philippe M. Chiasson" <>
Subject Re: [mp2] pools that go out of scope aren't a problem anymore?
Date Thu, 02 Dec 2004 07:04:54 GMT
Stas Bekman wrote:
> Stas Bekman wrote:
>> I've rebuit with --enable-pool-debug CPPFLAGS="-DAPR_BUCKET_DEBUG", 
>> but also apr1/httpd2.1
>>>>   % ~/perl/5.8.x/bin/perl -MApache2 -MAPR::Pool -MAPR::Table -wle '
>>>>   $t= APR::Table::make(APR::Pool->new, 10);   $t->set($_=>$_), print

>>>> "Set $_" for 1..20'
>>>>   Segmentation fault
>> Now I get the segfault

Yes, now I remember that the segv are usually happenning whith --enable-pool-debug
> So trying to go the way Joe has chosen for apreq, I've replaced the 
> plain apr_table_make autogenerated wrapper with explicit:
> static MP_INLINE SV *mpxs_APR__Table_make(pTHX_ SV *p_sv, int nelts)
> {
>     apr_pool_t *p = mp_xs_sv2_APR__Pool(p_sv);
>     apr_table_t *t = apr_table_make(p, nelts);
>     SV *t_sv = mp_xs_APR__Table_2obj(t);
>     sv_magic(SvRV(t_sv), p_sv, PERL_MAGIC_ext, Nullch, -1);
>     return t_sv;
> }
> and segfault is gone. here we create a dependency p_sv <- t_sv, so only 
> when t_sv goes out of scope the pool object will be destroyed, and this 
> code is now safe:
>   APR::Table::make(APR::Pool->new, 10);

Well, there remains the _other_ pool issue I caught the other day. This
_bad_ code example should illustrate.

my $pool;
sub handler {
  my $r = shift;
  $pool ||= $r->pool; #XXX: Yes, bad bad bad, but still...
  APR::Table::make($pool, 10); #bam!

The problem in this case is that apache destroyed the request pool at the end of the first
request. And we now have a valid $pool that now points to a freed pool. It's an incorrect
usage of the API, I know, but the resulting segv isn't nice...

I did start thinking about a solution to this kind of problem, but I am not sure it's a very
good suggestion. I was thinking along the way of _never_ handing off to Perl land SV wrappers
to external pool objects (i.e. httpd's), but instead, manage our own memory pools in parallel
and allow us to detect/trap such _bad_ usages.

For instance, for each request, we create a modperl_request_pool() and $r->pool returns
not the underlying $r->pool. We just register a cleanup handler with $r->pool to destroy
request pool, but use a refcount technique to figure out if the pool can be safely freed.
If not,
we either die verbosely, or just warn about it

  "Warning: prolonging the lifetime of request pool at foo/ line 232"

Certainly possible to implement, but I am just not sure we can justify this added magic/complexity
simply for the sake of catching improper uses of the API.

> Unfortunately it doesn't seem like this dependency code can be easily 
> integrated with the automatic type conversion, because the code accepts 
> a single argument in xs/modperl_xs_sv_convert.h.

I am not sure about this one, but couldn't we use our fancy XS generation framerowk
to magically detect methods with a pool argument and facilitate this somewhat ?

> At the moment we will need to rewrite all methods that accept the pool 
> object, like I did for APR::Table::make above. for example inside 
> APR::Table there are also copy() and overlay() that need the same solution.

And that sounds like a terrible idea to me, unless we can make it really tiny,
like a single macro or sth. Still, I think it would be much more error-prone than
figuring this out somewhat automatically. If the typemap stuff isn't sufficient,
why can't we just implement our own sort of meta-typemap from all the neat data
we got in xs/tables ?

> Any suggestions?
> I'm thinking whether we could use the same solution for PV return values 
> that use the pool object. For example with server_root_relative(), see 
> the end of notes at:
> which documents the danger of having the SvPV having a hole in body if 
> the pool went out of scope. I guess we can attach the same magical 
> dependency there too.

Yes, that the reverse problem of my example. And the same solution would possibly

Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5     F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5

View raw message