You are viewing a plain text version of this content. The canonical link for it is here.
Posted to modperl-cvs@perl.apache.org by ge...@apache.org on 2003/09/05 18:30:45 UTC
cvs commit: modperl-2.0/t/response/TestAPR pool.pm
geoff 2003/09/05 09:30:45
Modified: t/response/TestAPR pool.pm
Log:
add subpool tests
Revision Changes Path
1.4 +8 -1 modperl-2.0/t/response/TestAPR/pool.pm
Index: pool.pm
===================================================================
RCS file: /home/cvs/modperl-2.0/t/response/TestAPR/pool.pm,v
retrieving revision 1.3
retrieving revision 1.4
diff -u -r1.3 -r1.4
--- pool.pm 11 Apr 2002 11:08:44 -0000 1.3
+++ pool.pm 5 Sep 2003 16:30:45 -0000 1.4
@@ -17,18 +17,25 @@
sub handler {
my $r = shift;
- plan $r, tests => 2;
+ plan $r, tests => 4;
my $p = APR::Pool->new;
ok $p->isa('APR::Pool');
+ my $subp = $p->new;
+
+ ok $subp->isa('APR::Pool');
+
#only available with -DAPR_POOL_DEBUG
# my $num_bytes = $p->num_bytes;
# ok $num_bytes;
$p->cleanup_register(\&cleanup, 33);
+ $subp->cleanup_register(\&cleanup, 33);
+ # should destroy the subpool too, so
+ # cleanup is called twice
$p->destroy;
Apache::OK;
Re: cvs commit: modperl-2.0/t/response/TestAPR pool.pm
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>> +1, but could also do:
>>
>> sub cleanup {
>> my ($r, $what) = @{+shift};
>> $r->notes->add(cleanup => $what);
>> 1;
>> }
>> ...
>> $p->cleanup_register( \&cleanup, [$r, 'parent']);
>> $subp->cleanup_register(\&cleanup, [$r, 'child']);
>
>
> the idea was that one calls set and one calls add - although table order
> is guaranteed, so it shouldn't be an issue, if the parent pool is
> destroyed before the child then only one appears in notes (as opposed to
> them being in a different order).
and hence the test will fail. but yours is just fine.
also to make things foolproof you probably want to do:
($notes[0]||'') eq $whatever
otherwise it may die on undefined...
__________________________________________________________________
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
Re: cvs commit: modperl-2.0/t/response/TestAPR pool.pm
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
Stas Bekman wrote:
> Geoffrey Young wrote:
>
>>
>>> it's probably better to have a new 'sub cleanup_sub_pool', otherwise
>>> how do we know that it's not the main pool that gets destroyed twice?
>>>
>>>> + # should destroy the subpool too, so
>>>> + # cleanup is called twice
>>>> $p->destroy;
>>>
>>>
>>>
>>>
>>> also should we test that a sub-pool is destroyed before the main pool?
>>
>>
>>
>> how about this
>
>
> +1, but could also do:
>
> sub cleanup {
> my ($r, $what) = @{+shift};
> $r->notes->add(cleanup => $what);
> 1;
> }
> ...
> $p->cleanup_register( \&cleanup, [$r, 'parent']);
> $subp->cleanup_register(\&cleanup, [$r, 'child']);
the idea was that one calls set and one calls add - although table order is
guaranteed, so it shouldn't be an issue, if the parent pool is destroyed
before the child then only one appears in notes (as opposed to them being in
a different order).
>
> either way is good, just trying to put new features to stress ;)
>
> also will this call destroy on $p on the exit from { } ?
>
> {
> my $p = APR::Pool->new;
> }
>
> I think it should.
I'll add that too.
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: cvs commit: modperl-2.0/t/response/TestAPR pool.pm
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>> Do you think it should be here, as there is no apr_pool_DESTROY
>> function. I'd think that it should be in modperl_functions, but may be
>> putting all related mappings together is better.
>>
>> After all this is the same:
>>
>> xs/maps/apr_functions.map: apr_uuid_DESTROY
>
>
> it doesn't matter to me. I was just trying to get something working
> first - we can shuffle and tweak the implementation after it is doing
> what we want.
Sure, let's keep it together with all the apr_pool functions for now.
__________________________________________________________________
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
Re: cvs commit: modperl-2.0/t/response/TestAPR pool.pm
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
>> well, maybe. if we go destroying major pools (request, server) before
>> it's time that could be bad. and if $r->pool is paired to an
>> APR::Pool object, and $r goes out of scope, that would call
>> APR::Pool::DESTROY, right?
>
>
> OK, I see what kind of worms can it opens. r->pool is not an SV*, so it
> won't get destroyed by perl's DESTROY but by httpd internals at the end
> of the request. However if you do: my $r = $r->pool anywhere in the
> code, its reference count will be one, and it'll be DESTROYED at the end
> of the scope, messing things up.
>
> So we want DESTROY to work for privately created pools, but not for
> internal pools, that gets tricky. One solution is to have
> APR::Pool::new() do something special to an SV that it creates, e.g. set
> some flag or attach magic. When DESTROY is called it should do its work
> only on those SVs marked as DESTROY-able.
cool. I'll try and do something like that.
> Do you think it should be here, as there is no apr_pool_DESTROY
> function. I'd think that it should be in modperl_functions, but may be
> putting all related mappings together is better.
>
> After all this is the same:
>
> xs/maps/apr_functions.map: apr_uuid_DESTROY
it doesn't matter to me. I was just trying to get something working first -
we can shuffle and tweak the implementation after it is doing what we want.
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: cvs commit: modperl-2.0/t/response/TestAPR pool.pm
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>>> I fixed things so that DESTROY was available, but it caused all kinds
>>> of problems with the test suite, so I'm guessing it's a good thing
>>> that pools need to be explicitly destroyed.
>>
>>
>>
>> what problems?
>
>
> all kinds of problems. like random modules in the test suite start to
> fail or hang. unless I got the fix wrong (below), but it was the only
> way I could see to create the method, since the #define clearly isn't
> working (which is interesting, since there are two other classes that
> only use the alias as well).
>
>> it's just an object, I suppose that it should just work.
>
>
> well, maybe. if we go destroying major pools (request, server) before
> it's time that could be bad. and if $r->pool is paired to an APR::Pool
> object, and $r goes out of scope, that would call APR::Pool::DESTROY,
> right?
OK, I see what kind of worms can it opens. r->pool is not an SV*, so it won't
get destroyed by perl's DESTROY but by httpd internals at the end of the
request. However if you do: my $r = $r->pool anywhere in the code, its
reference count will be one, and it'll be DESTROYED at the end of the scope,
messing things up.
So we want DESTROY to work for privately created pools, but not for internal
pools, that gets tricky. One solution is to have APR::Pool::new() do something
special to an SV that it creates, e.g. set some flag or attach magic. When
DESTROY is called it should do its work only on those SVs marked as DESTROY-able.
> Index: xs/maps/apr_functions.map
> ===================================================================
> RCS file: /home/cvspublic/modperl-2.0/xs/maps/apr_functions.map,v
> retrieving revision 1.57
> diff -u -r1.57 apr_functions.map
> --- xs/maps/apr_functions.map 4 Sep 2003 16:39:44 -0000 1.57
> +++ xs/maps/apr_functions.map 5 Sep 2003 19:27:00 -0000
> @@ -155,6 +155,7 @@
> apr_pool_clear
> >apr_pool_clear_debug
> apr_pool_destroy
> + apr_pool_DESTROY
Do you think it should be here, as there is no apr_pool_DESTROY function. I'd
think that it should be in modperl_functions, but may be putting all related
mappings together is better.
After all this is the same:
xs/maps/apr_functions.map: apr_uuid_DESTROY
__________________________________________________________________
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
Re: cvs commit: modperl-2.0/t/response/TestAPR pool.pm
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
Stas Bekman wrote:
> Geoffrey Young wrote:
>
>> ither way is good, just trying to put new features to stress ;)
>>
>>>
>>> also will this call destroy on $p on the exit from { } ?
>>>
>>> {
>>> my $p = APR::Pool->new;
>>> }
>>>
>>> I think it should.
>>
>>
>>
>> so do I, but it doesn't. you can't call $p->DESTROY either, even
>> though the alias is there in APR__Pool.h
>
>
> so it's broken.
well...
>
>> I fixed things so that DESTROY was available, but it caused all kinds
>> of problems with the test suite, so I'm guessing it's a good thing
>> that pools need to be explicitly destroyed.
>
>
> what problems?
all kinds of problems. like random modules in the test suite start to fail
or hang. unless I got the fix wrong (below), but it was the only way I
could see to create the method, since the #define clearly isn't working
(which is interesting, since there are two other classes that only use the
alias as well).
> it's just an object, I suppose that it should just work.
well, maybe. if we go destroying major pools (request, server) before it's
time that could be bad. and if $r->pool is paired to an APR::Pool object,
and $r goes out of scope, that would call APR::Pool::DESTROY, right?
--Geoff
Index: xs/maps/apr_functions.map
===================================================================
RCS file: /home/cvspublic/modperl-2.0/xs/maps/apr_functions.map,v
retrieving revision 1.57
diff -u -r1.57 apr_functions.map
--- xs/maps/apr_functions.map 4 Sep 2003 16:39:44 -0000 1.57
+++ xs/maps/apr_functions.map 5 Sep 2003 19:27:00 -0000
@@ -155,6 +155,7 @@
apr_pool_clear
>apr_pool_clear_debug
apr_pool_destroy
+ apr_pool_DESTROY
>apr_pool_destroy_debug
apr_pool_t *:DEFINE_new | mpxs_apr_pool_create | SV *:obj
-apr_pool_create_ex
cvs server: Diffing xs/tables
cvs server: Diffing xs/tables/current
cvs server: Diffing xs/tables/current/Apache
Index: xs/tables/current/Apache/FunctionTable.pm
===================================================================
RCS file:
/home/cvspublic/modperl-2.0/xs/tables/current/Apache/FunctionTable.pm,v
retrieving revision 1.47
diff -u -r1.47 FunctionTable.pm
--- xs/tables/current/Apache/FunctionTable.pm 29 Aug 2003 14:31:00 -0000
1.47
+++ xs/tables/current/Apache/FunctionTable.pm 5 Sep 2003 19:27:08 -0000
@@ -10040,6 +10040,16 @@
},
{
'return_type' => 'void',
+ 'name' => 'apr_pool_DESTROY',
+ 'args' => [
+ {
+ 'type' => 'apr_pool_t *',
+ 'name' => 'p'
+ }
+ ]
+ },
+ {
+ 'return_type' => 'void',
'name' => 'apr_pool_destroy_debug',
'args' => [
{
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: cvs commit: modperl-2.0/t/response/TestAPR pool.pm
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
> ither way is good, just trying to put new features to stress ;)
>
>>
>> also will this call destroy on $p on the exit from { } ?
>>
>> {
>> my $p = APR::Pool->new;
>> }
>>
>> I think it should.
>
>
> so do I, but it doesn't. you can't call $p->DESTROY either, even though
> the alias is there in APR__Pool.h
so it's broken.
> I fixed things so that DESTROY was available, but it caused all kinds of
> problems with the test suite, so I'm guessing it's a good thing that
> pools need to be explicitly destroyed.
what problems? it's just an object, I suppose that it should just work.
__________________________________________________________________
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
Re: cvs commit: modperl-2.0/t/response/TestAPR pool.pm
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
ither way is good, just trying to put new features to stress ;)
>
> also will this call destroy on $p on the exit from { } ?
>
> {
> my $p = APR::Pool->new;
> }
>
> I think it should.
so do I, but it doesn't. you can't call $p->DESTROY either, even though the
alias is there in APR__Pool.h
I fixed things so that DESTROY was available, but it caused all kinds of
problems with the test suite, so I'm guessing it's a good thing that pools
need to be explicitly destroyed.
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: cvs commit: modperl-2.0/t/response/TestAPR pool.pm
Posted by Stas Bekman <st...@stason.org>.
Geoffrey Young wrote:
>
>> it's probably better to have a new 'sub cleanup_sub_pool', otherwise
>> how do we know that it's not the main pool that gets destroyed twice?
>>
>>> + # should destroy the subpool too, so
>>> + # cleanup is called twice
>>> $p->destroy;
>>
>>
>>
>> also should we test that a sub-pool is destroyed before the main pool?
>
>
> how about this
+1, but could also do:
sub cleanup {
my ($r, $what) = @{+shift};
$r->notes->add(cleanup => $what);
1;
}
...
$p->cleanup_register( \&cleanup, [$r, 'parent']);
$subp->cleanup_register(\&cleanup, [$r, 'child']);
either way is good, just trying to put new features to stress ;)
also will this call destroy on $p on the exit from { } ?
{
my $p = APR::Pool->new;
}
I think it should.
__________________________________________________________________
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
Re: cvs commit: modperl-2.0/t/response/TestAPR pool.pm
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> it's probably better to have a new 'sub cleanup_sub_pool', otherwise how
> do we know that it's not the main pool that gets destroyed twice?
>
>> + # should destroy the subpool too, so
>> + # cleanup is called twice
>> $p->destroy;
>
>
> also should we test that a sub-pool is destroyed before the main pool?
how about this
--- t/response/TestAPR/pool.pm 5 Sep 2003 16:30:45 -0000 1.4
+++ t/response/TestAPR/pool.pm 5 Sep 2003 17:34:47 -0000
@@ -5,13 +5,20 @@
use Apache::Test;
+use Apache::RequestRec ();
use APR::Pool ();
+use APR::Table ();
use Apache::Const -compile => 'OK';
-sub cleanup {
- my $arg = shift;
- ok $arg == 33;
+sub parent_cleanup {
+ shift->notes->add(cleanup => 'parent');
+ 1;
+}
+
+sub child_cleanup {
+ shift->notes->set(cleanup => 'child');
+ 1;
}
sub handler {
@@ -31,12 +38,15 @@
# my $num_bytes = $p->num_bytes;
# ok $num_bytes;
- $p->cleanup_register(\&cleanup, 33);
- $subp->cleanup_register(\&cleanup, 33);
+ $p->cleanup_register(\&parent_cleanup, $r);
+ $subp->cleanup_register(\&child_cleanup, $r);
- # should destroy the subpool too, so
- # cleanup is called twice
+ # should destroy the subpool too
$p->destroy;
+
+ my @notes = $r->notes->get('cleanup');
+ ok $notes[0] eq 'child';
+ ok $notes[1] eq 'parent';
Apache::OK;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: cvs commit: modperl-2.0/t/response/TestAPR pool.pm
Posted by Stas Bekman <st...@stason.org>.
geoff@apache.org wrote:
> geoff 2003/09/05 09:30:45
>
> Modified: t/response/TestAPR pool.pm
> Log:
> add subpool tests
> + my $subp = $p->new;
> +
> + ok $subp->isa('APR::Pool');
> +
> #only available with -DAPR_POOL_DEBUG
> # my $num_bytes = $p->num_bytes;
> # ok $num_bytes;
>
> $p->cleanup_register(\&cleanup, 33);
> + $subp->cleanup_register(\&cleanup, 33);
it's probably better to have a new 'sub cleanup_sub_pool', otherwise how do we
know that it's not the main pool that gets destroyed twice?
> + # should destroy the subpool too, so
> + # cleanup is called twice
> $p->destroy;
also should we test that a sub-pool is destroyed before the main pool?
__________________________________________________________________
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