You are viewing a plain text version of this content. The canonical link for it is here.
Posted to apreq-dev@httpd.apache.org by Stas Bekman <st...@stason.org> on 2003/11/13 03:01:56 UTC
Re: [MP1] Apache segfault after serving request
Further reduced to the minimal:
<Location /crash2>
SetHandler perl-script
PerlHandler Server::Killer
</Location>
package Server::Killer;
use 5.008;
use strict;
use Apache::Request;
use Apache::Constants qw(:common);
our $GlobalS;
sub handler
{
my $r = shift;
$r->send_http_header;
$r->print("ok");
my $q=Apache::Request->new($r);
$GlobalS=$q->param();
return OK;
}
1;
This patch against the current cvs prevents the segfault. Though it doesn't
eliminate the problem, since it's caused elsewhere.
Index: src/modules/perl/perl_util.c
===================================================================
RCS file: /home/cvs/modperl/src/modules/perl/perl_util.c,v
retrieving revision 1.53
diff -u -r1.53 perl_util.c
--- src/modules/perl/perl_util.c 30 Oct 2003 19:39:17 -0000 1.53
+++ src/modules/perl/perl_util.c 13 Nov 2003 01:33:46 -0000
@@ -96,6 +96,7 @@
if(SvROK(rv) && SvTYPE(SvRV(rv)) == SVt_PVHV) {
SV *sv = perl_hvrv_magic_obj(rv);
if(!sv) croak("HV is not magic!");
+ if(!SvROK(sv)) croak("trying to free magic that was freed already!");
return (table *)SvIV((SV*)SvRV(sv));
}
return (table *)SvIV((SV*)SvRV(rv));
It seems that your global $GlobalS var contains a reference to an
Apache::Request table, but A-R object is not global, so when the scope of the
handler is exited A-R object gets destroyed, leaving $GlobalS containting a
pointer to a black hole. That black hole is req->parms in Request.xs in libapreq.
And here is a quick workaround for you, while we are figuring out the origin
problem:
$r->push_handlers(PerlCleanupHandler =>
sub { undef $GlobalS if defined $GlobalS } );
So the new handler now looks like:
package Server::Killer;
use 5.008;
use strict;
use Apache::Request;
use Apache::Constants qw(:common);
our $GlobalS;
sub handler
{
my $r = shift;
$r->send_http_header;
$r->print("ok");
my $q=Apache::Request->new($r);
$GlobalS=$q->param();
$r->push_handlers(PerlCleanupHandler =>
sub { undef $GlobalS if defined $GlobalS } );
return OK;
}
1;
Actually a simple:
undef $GlobalS;
return OK;
}
does the trick. Again, what you want to achieve is destroying that global
reference to parm before A-R object is destroyed.
Though, since in your original code you import the global symbols you will
need to destroy them in the package they were created. But this will do the trick:
$GlobalS = undef;
return OK;
}
as it'll affect the alias as well as the exported variable, putting the
reference counting to 0.
Joe, any ideas on this one? (we are talking about libapreq1/mp1) May be we
should just assume that it's OK that the A-R table was already freed? Does
this patch introduce a leak? I'd suggest to write a test for libapreq2 to
cover this case.
Index: src/modules/perl/perl_util.c
===================================================================
RCS file: /home/cvs/modperl/src/modules/perl/perl_util.c,v
retrieving revision 1.53
diff -u -r1.53 perl_util.c
--- src/modules/perl/perl_util.c 30 Oct 2003 19:39:17 -0000 1.53
+++ src/modules/perl/perl_util.c 13 Nov 2003 01:49:48 -0000
@@ -96,6 +96,8 @@
if(SvROK(rv) && SvTYPE(SvRV(rv)) == SVt_PVHV) {
SV *sv = perl_hvrv_magic_obj(rv);
if(!sv) croak("HV is not magic!");
+ /* already freed? */
+ if(!SvROK(sv)) return (table *)NULL;
return (table *)SvIV((SV*)SvRV(sv));
}
return (table *)SvIV((SV*)SvRV(rv));
Index: src/modules/perl/Table.xs
===================================================================
RCS file: /home/cvs/modperl/src/modules/perl/Table.xs,v
retrieving revision 1.10
diff -u -r1.10 Table.xs
--- src/modules/perl/Table.xs 23 May 2000 15:56:12 -0000 1.10
+++ src/modules/perl/Table.xs 13 Nov 2003 01:49:48 -0000
@@ -114,9 +114,13 @@
Apache__Table tab;
CODE:
- tab = (Apache__Table)hvrv2table(self);
- if(SvROK(self) && SvTYPE(SvRV(self)) == SVt_PVHV)
- safefree(tab);
+ if(self && SvROK(self) && SvTYPE(SvRV(self)) == SVt_PVHV) {
+ tab = (Apache__Table)hvrv2table(self);
+ if (tab) {
+ safefree(tab);
+
+ }
+ }
void
FETCH(self, key)
__________________________________________________________________
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
Re: [MP1] Apache segfault after serving request
Posted by Marc Slagle <ma...@whapps.com>.
> ALso, Marc, I'd suggest to rethink your coding techniques. Instead of using
> globals you should probably encapsulate all the data that you want to pass
> around into a single object and and pass it around, or using a Singleton
> object. Globals are not the best way to go in majority of cases.
Thanks for all the help. We'll be looking into the Singleton object
today. I agree that globals are not the best way to go. We will
attempt to get our examples as small as possible if we need to post in
the future.
Marc Slagle
Whapps, LLC
--
Reporting bugs: http://perl.apache.org/bugs/
Mail list info: http://perl.apache.org/maillist/modperl.html
Re: [MP1] Apache segfault after serving request
Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
> Stas Bekman <st...@stason.org> writes:
>
> [...]
>
>
>>It seems that your global $GlobalS var contains a reference to an
>>Apache::Request table, but A-R object is not global, so when the scope
>>of the handler is exited A-R object gets destroyed, leaving $GlobalS
>>containting a pointer to a black hole. That black hole is req->parms
>>in Request.xs in libapreq.
>
>
> <yack>
> Yup- the request pool's cleanup hook has already come through and
> reclaimed all those C data structures. Unfortunately the perl glue
> isn't aware that this happened.
>
> I think the basic assumption is that the request pool's lifetime must
> be longer than any perl scalar glued to a per-request struct. Of
> course, that assumption doesn't appear to be valid in this situation
> (a global Apache::Table object is alive, but the associated C table was
> freed). Not sure what to do about it right now.
> </yack>
We have exactly the same issue with APR::Pool objects created via
my $parent_pool = APR::Pool->new();
my $child_pool = $parent_pool->new();
if $parent_pool is destroyed first, it also eats the guts of its child,
leaving $child_pool pointing to some invalid thing. The worst thing is that if
some other pool gets allocated at the same pointer where child_pool point, it
will be destroyed!
Moreover, every time you say:
my $parent2 = $child_pool->parent_get;
you can't destroy $parent2, because it will nuke the guts of the parent and
all the children. Therefore you can't just wrap and IV pointer into an SV and
hand it to the perl side.
Currently the solution that almost works is to introduce an internal reference
counting, by incrementing the reference counting every time a new perl
variable pointing to the pool is created, and decrementing when it's attempted
to be destroyed. Only when the reference count reaches 0 we can destroy the
actual pool. This reference counting is stored inside the C pool struct
itself, via user data API.
It's almost working. There is at least one problem with this approach that I
forgot what it was, but I have a test that demonstrates it somewhere. The
solution that I'm thinking to implement involves storing a real SV in the
pool's user data and use Perl's reference counting.
I think the same approach can be taken by A-R. Once I'll polish the solution
with APR::Pool, we can apply it to A-R. In any case I've provided Marc with a
workaround which should prevent segfaults, so there is no rush to fix it if
you are busy with more urgent things.
ALso, Marc, I'd suggest to rethink your coding techniques. Instead of using
globals you should probably encapsulate all the data that you want to pass
around into a single object and and pass it around, or using a Singleton
object. Globals are not the best way to go in majority of cases.
__________________________________________________________________
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
--
Reporting bugs: http://perl.apache.org/bugs/
Mail list info: http://perl.apache.org/maillist/modperl.html
Re: [MP1] Apache segfault after serving request
Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
> Stas Bekman <st...@stason.org> writes:
>
> [...]
>
>
>>It seems that your global $GlobalS var contains a reference to an
>>Apache::Request table, but A-R object is not global, so when the scope
>>of the handler is exited A-R object gets destroyed, leaving $GlobalS
>>containting a pointer to a black hole. That black hole is req->parms
>>in Request.xs in libapreq.
>
>
> <yack>
> Yup- the request pool's cleanup hook has already come through and
> reclaimed all those C data structures. Unfortunately the perl glue
> isn't aware that this happened.
>
> I think the basic assumption is that the request pool's lifetime must
> be longer than any perl scalar glued to a per-request struct. Of
> course, that assumption doesn't appear to be valid in this situation
> (a global Apache::Table object is alive, but the associated C table was
> freed). Not sure what to do about it right now.
> </yack>
We have exactly the same issue with APR::Pool objects created via
my $parent_pool = APR::Pool->new();
my $child_pool = $parent_pool->new();
if $parent_pool is destroyed first, it also eats the guts of its child,
leaving $child_pool pointing to some invalid thing. The worst thing is that if
some other pool gets allocated at the same pointer where child_pool point, it
will be destroyed!
Moreover, every time you say:
my $parent2 = $child_pool->parent_get;
you can't destroy $parent2, because it will nuke the guts of the parent and
all the children. Therefore you can't just wrap and IV pointer into an SV and
hand it to the perl side.
Currently the solution that almost works is to introduce an internal reference
counting, by incrementing the reference counting every time a new perl
variable pointing to the pool is created, and decrementing when it's attempted
to be destroyed. Only when the reference count reaches 0 we can destroy the
actual pool. This reference counting is stored inside the C pool struct
itself, via user data API.
It's almost working. There is at least one problem with this approach that I
forgot what it was, but I have a test that demonstrates it somewhere. The
solution that I'm thinking to implement involves storing a real SV in the
pool's user data and use Perl's reference counting.
I think the same approach can be taken by A-R. Once I'll polish the solution
with APR::Pool, we can apply it to A-R. In any case I've provided Marc with a
workaround which should prevent segfaults, so there is no rush to fix it if
you are busy with more urgent things.
ALso, Marc, I'd suggest to rethink your coding techniques. Instead of using
globals you should probably encapsulate all the data that you want to pass
around into a single object and and pass it around, or using a Singleton
object. Globals are not the best way to go in majority of cases.
__________________________________________________________________
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
Re: [MP1] Apache segfault after serving request
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Stas Bekman <st...@stason.org> writes:
[...]
> It seems that your global $GlobalS var contains a reference to an
> Apache::Request table, but A-R object is not global, so when the scope
> of the handler is exited A-R object gets destroyed, leaving $GlobalS
> containting a pointer to a black hole. That black hole is req->parms
> in Request.xs in libapreq.
<yack>
Yup- the request pool's cleanup hook has already come through and
reclaimed all those C data structures. Unfortunately the perl glue
isn't aware that this happened.
I think the basic assumption is that the request pool's lifetime must
be longer than any perl scalar glued to a per-request struct. Of
course, that assumption doesn't appear to be valid in this situation
(a global Apache::Table object is alive, but the associated C table was
freed). Not sure what to do about it right now.
</yack>
[...]
> Joe, any ideas on this one? (we are talking about libapreq1/mp1) May
> be we should just assume that it's OK that the A-R table was already
> freed? Does this patch introduce a leak?
I don't know- I'll try to think about it more carefully before the week
is over.
Thanks, Stas.
--
Joe Schaefer