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 that, not the underlying $r->pool. We just register a cleanup handler with $r->pool to destroy our 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/bar.pl 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:
http://perl.apache.org/docs/2.0/api/Apache/ServerUtil.html#C_server_root_relative_
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 apply.
--------------------------------------------------------------------------------
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5 http://gozer.ectoplasm.org/ F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5
signature.asc
Description: OpenPGP digital signature
