You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by "Philippe M. Chiasson" <go...@cpan.org> on 2001/10/30 08:37:27 UTC
[Patch] stubs for cleaning up of r, scfg and dcfg
Following is a simple patch to use apr_pool_cleanup_register to the few structures
we allocate dynamically and where there might be stuff we want to clean up right
before the pool system frees it.
This would allow me, for example, to change my $r->pnotes patch so
it doesn't leak.
/home/gozer/sources/mod_perl2/deps/perl-12358/bin/perl build/cvsdiff
Index: src/modules/perl/modperl_config.c
===================================================================
RCS file: /home/anoncvs/mod_perl-2-cvs/src/modules/perl/modperl_config.c,v
retrieving revision 1.44
diff -u -I'$Id' -I'$Revision' -r1.44 modperl_config.c
--- src/modules/perl/modperl_config.c 22 Oct 2001 17:20:11 -0000 1.44
+++ src/modules/perl/modperl_config.c 30 Oct 2001 07:36:42 -0000
@@ -75,6 +75,10 @@
modperl_config_req_t *rcfg =
(modperl_config_req_t *)apr_pcalloc(r->pool, sizeof(*rcfg));
+ apr_pool_cleanup_register(r->pool, rcfg,
+ apr_pool_cleanup_null,
+ apr_pool_cleanup_null);
+
rcfg->wbucket.r = r;
MP_TRACE_d(MP_FUNC, "0x%lx\n", (unsigned long)rcfg);
@@ -87,6 +91,10 @@
modperl_config_srv_t *scfg = (modperl_config_srv_t *)
apr_pcalloc(p, sizeof(*scfg));
+ apr_pool_cleanup_register(p, scfg,
+ apr_pool_cleanup_null,
+ apr_pool_cleanup_null);
+
scfg->flags = modperl_options_new(p, MpSrvType);
MpSrvENABLE_On(scfg); /* mod_perl enabled by default */
MpSrvHOOKS_ALL_On(scfg); /* all hooks enabled by default */
@@ -110,6 +118,10 @@
modperl_config_dir_t *dcfg = (modperl_config_dir_t *)
apr_pcalloc(p, sizeof(modperl_config_dir_t));
+ apr_pool_cleanup_register(p, dcfg,
+ apr_pool_cleanup_null,
+ apr_pool_cleanup_null);
+
dcfg->flags = modperl_options_new(p, MpDirType);
dcfg->SetVar = apr_table_make(p, 2);
--
Philippe M. Chiasson <go...@cpan.org>
Extropia's Resident System Guru
http://www.eXtropia.com/
/* When we have more time, we can teach the penguin to say
* "By your command" or "Activating turbo boost, Michael".
*/
-- Linux 2.2.16
/usr/src/linux/arch/sparc/prom/sun4prom.c
perl -e '$$=\${gozer};{$_=unpack(P26,pack(L,$$));/^Just Another Perl Hacker!\n$/&&print||$$++&&redo}'
Re: [Patch] stubs for cleaning up of r, scfg and dcfg
Posted by "Philippe M. Chiasson" <go...@cpan.org>.
On Sun, Nov 04, 2001 at 09:36:08PM -0800, Doug MacEachern wrote:
> try current cvs, there is now a per-request cleanup function
> modperl_config.c:modperl_config_request_cleanup()
> that already clears rcfg->pnotes if it exists and can be used for any
> other per-request cleanups.
Right on, will check it out as soon as I can. Expect a corrected
$r->pnotes anytime after that.
>
--
+----------------------------------------------------+
| Philippe M. Chiasson <go...@cpan.org> |
+----------------------------------------------------+
| F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3 A5A5 |
+----------------------------------------------------+
gethostent not implemented : Your C library apparently
doesn't implement gethostent(), probably because if it did,
it'd feel morally obligated to return every hostname on the
Internet.
-- perldiag(1)
perl -e '$$=\${gozer};{$_=unpack(P26,pack(L,$$));/^Just Another Perl Hacker!\n$/&&print||$$++&&redo}'
Re: [Patch] stubs for cleaning up of r, scfg and dcfg
Posted by Doug MacEachern <do...@covalent.net>.
try current cvs, there is now a per-request cleanup function
modperl_config.c:modperl_config_request_cleanup()
that already clears rcfg->pnotes if it exists and can be used for any
other per-request cleanups.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [Patch] stubs for cleaning up of r, scfg and dcfg
Posted by Doug MacEachern <do...@covalent.net>.
On Tue, 30 Oct 2001, Philippe M. Chiasson wrote:
> Actually, now that I look at it, I started to dislike this solution already, sorry.
> Whenever some module wants to associate something in a request/server/dir object,
> you'd have to add cleanup code to one of the cleanup_register functions.
>
> Would it be okay for something like $r->pnotes to directly register it's own
> cleanup function when or if it has to create a new HV to free it later ?
>
> Or if this is likely to happen a lot, would it make sense to abstract this
> a bit more and provide a more generic way to register the stuff you created
> with malloc and make sure it's automatically freed whenever it should be ?
>
> Here is an example of a working, not leaking pnotes implementation that cleans
> up after itself. If you ask me, it's a tiny bit more complicated then it should be,
> but here it is.
thanks, but don't forget the discussion below. i haven't had a chance to
finish implementing the interpreter pool cleanup mechanism. hopefully
will soon and it'll do the trick for properly cleaning up pnotes.
Date: Thu, 11 Oct 2001 08:50:58 -0700 (PDT)
From: Doug MacEachern <do...@covalent.net>
To: "Philippe M. Chiasson" <go...@cpan.org>
cc: dev@perl.apache.org
Subject: Re: Timing/location for proper rcfg cleanup ?
In-Reply-To: <20...@eXtropia.com>
Message-ID: <Pi...@mako.covalent.net>
On Thu, 11 Oct 2001, Philippe M. Chiasson wrote:
> Thanks, and might that be slightly more usefull in a macro like :
ok, i've added one for the moment. but in the future, i hope nobody needs
it. (see below)
> Good. But I discovered another slight annoyance I can't quite resolve. ap_hook_create_request
> is used to create 'r' for every request, I guessed, but there doesn't seem to be a clean way
> to handle destruction/cleaning of that request. So now I am left wondering when/how to step
> in just before a request obj is de-allocated, to free stuff that wasn't allocated with apr_pools.
you can use a cleanup in r->pool. however, i'm thinking in general there
should be a cleanup mechanism for the interpreter pool, so cleanups can
happen when the interpreter is put back into the pool. because at the
moment, pnotes will break if PerlInterpScope is configured to handler or
subrequest. because the interpreter will have already been put back into
the pool by the time r->pool cleanups are run in these cases.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [Patch] stubs for cleaning up of r, scfg and dcfg
Posted by "Philippe M. Chiasson" <go...@cpan.org>.
On Tue, Oct 30, 2001 at 03:37:27PM +0800, Philippe M. Chiasson wrote:
> Following is a simple patch to use apr_pool_cleanup_register to the few structures
> we allocate dynamically and where there might be stuff we want to clean up right
> before the pool system frees it.
>
> This would allow me, for example, to change my $r->pnotes patch so
> it doesn't leak.
Actually, now that I look at it, I started to dislike this solution already, sorry.
Whenever some module wants to associate something in a request/server/dir object,
you'd have to add cleanup code to one of the cleanup_register functions.
Would it be okay for something like $r->pnotes to directly register it's own
cleanup function when or if it has to create a new HV to free it later ?
Or if this is likely to happen a lot, would it make sense to abstract this
a bit more and provide a more generic way to register the stuff you created
with malloc and make sure it's automatically freed whenever it should be ?
Here is an example of a working, not leaking pnotes implementation that cleans
up after itself. If you ask me, it's a tiny bit more complicated then it should be,
but here it is.
/home/gozer/sources/mod_perl2/deps/perl-12358/bin/perl build/cvsdiff
Binary files /dev/null and t/core.17281 differ
Index: docs/src/api/mod_perl-2.0/Apache/RequestRec.pod
===================================================================
RCS file: /home/anoncvs/mod_perl-docs-cvs/src/api/mod_perl-2.0/Apache/RequestRec.pod,v
retrieving revision 1.2
diff -u -I'$Id' -I'$Revision' -r1.2 RequestRec.pod
--- docs/src/api/mod_perl-2.0/Apache/RequestRec.pod 10 Oct 2001 05:06:36 -0000 1.2
+++ docs/src/api/mod_perl-2.0/Apache/RequestRec.pod 30 Oct 2001 09:00:40 -0000
@@ -75,6 +75,20 @@
values associated with the key C<$key> (and the key itself) will be
deleted.
+=item * $r->pnotes( [ $key, [$value] ] )
+
+Like $r->notes, but takes any scalar as an value.
+
+ $r->pnotes("MY_HANDLER" => [qw(one two)]);
+ my $val = $r->pnotes("MY_HANDLER");
+ print $val->[0]; # prints "one"
+
+ my $hashref = $r->pnotes();
+ print $hashref->{'MY_HANDLER'}[0]; #also prints one
+
+Advantage over just using a Perl variable is that $r->pnotes gets
+cleaned up after every request.
+
=item *
=back
Index: t/response/TestAPI/request_rec.pm
===================================================================
RCS file: /home/anoncvs/mod_perl-2-cvs/t/response/TestAPI/request_rec.pm,v
retrieving revision 1.8
diff -u -I'$Id' -I'$Revision' -r1.8 request_rec.pm
--- t/response/TestAPI/request_rec.pm 28 Sep 2001 20:11:02 -0000 1.8
+++ t/response/TestAPI/request_rec.pm 30 Oct 2001 09:00:40 -0000
@@ -11,7 +11,7 @@
sub handler {
my $r = shift;
- plan $r, tests => 48;
+ plan $r, tests => 53;
#Apache->request($r); #PerlOptions +GlobalRequest takes care
my $gr = Apache->request;
@@ -78,7 +78,17 @@
ok $r->subprocess_env;
ok $r->notes;
-
+
+ ok $r->pnotes;
+
+ ok t_cmp('HASH', ref($r->pnotes), qq{ref($r->pnotes)});
+
+ ok t_cmp(undef, $r->pnotes('pnotes_foo','pnotes_bar'), qq{\$r->pnotes(key,val)});
+
+ ok t_cmp("pnotes_bar", $r->pnotes('pnotes_foo') , qq{\$r->pnotes(key)});
+
+ ok t_cmp('pnotes_bar', $r->pnotes()->{'pnotes_foo'}, qq{\$r->pnotes()->{}});
+
ok $r->content_type;
ok $r->handler;
Index: todo/api.txt
===================================================================
RCS file: /home/anoncvs/mod_perl-2-cvs/todo/api.txt,v
retrieving revision 1.11
diff -u -I'$Id' -I'$Revision' -r1.11 api.txt
--- todo/api.txt 9 Oct 2001 00:14:10 -0000 1.11
+++ todo/api.txt 30 Oct 2001 09:00:40 -0000
@@ -9,9 +9,6 @@
need apr_finfo_t <-> struct stat conversion (might already be there,
haven't looked close enough yet)
-$r->pnotes:
-not yet implemented
-
$r->subprocess_env:
in void context should populate %ENV as 1.x does
Index: xs/Apache/RequestUtil/Apache__RequestUtil.h
===================================================================
RCS file: /home/anoncvs/mod_perl-2-cvs/xs/Apache/RequestUtil/Apache__RequestUtil.h,v
retrieving revision 1.7
diff -u -I'$Id' -I'$Revision' -r1.7 Apache__RequestUtil.h
--- xs/Apache/RequestUtil/Apache__RequestUtil.h 28 Sep 2001 20:11:02 -0000 1.7
+++ xs/Apache/RequestUtil/Apache__RequestUtil.h 30 Oct 2001 09:00:40 -0000
@@ -177,5 +177,62 @@
return retval;
}
+static MP_INLINE
+apr_status_t modperl_pnotes_free(void *data)
+{
+ request_rec *r = (request_rec *)data;
+ MP_dRCFG;
+ MP_dTHX;
+
+ if (rcfg->pnotes) {
+ hv_clear(rcfg->pnotes);
+ SvREFCNT_dec(rcfg->pnotes);
+ rcfg->pnotes = NULL;
+ }
+
+ return APR_SUCCESS;
+}
+
+static MP_INLINE
+SV *mpxs_Apache__RequestRec_pnotes(pTHX_ request_rec *r, SV *k, SV *val)
+{
+ MP_dRCFG;
+ char *key = NULL;
+ SV *retval = &PL_sv_undef;
+ STRLEN len;
+
+ if (!rcfg) {
+ return retval;
+ }
+
+ if (!rcfg->pnotes) {
+ rcfg->pnotes = newHV();
+ apr_pool_cleanup_register(r->pool, r,
+ modperl_pnotes_free,
+ apr_pool_cleanup_null);
+ }
+
+ if (k) {
+ key = SvPV(k, len);
+ }
+
+ if (key) {
+ if (hv_exists(rcfg->pnotes, key, len)) {
+ retval = SvREFCNT_inc(*hv_fetch(rcfg->pnotes, key, len, FALSE));
+ sv_2mortal(retval);
+ }
+
+ if (val) {
+ hv_store(rcfg->pnotes, key, len, SvREFCNT_inc(val), FALSE);
+ }
+ }
+ else {
+ retval = newRV_inc((SV *)rcfg->pnotes);
+ SvREFCNT_inc(sv_2mortal(retval));
+ }
+
+ return retval;
+}
+
#define mpxs_Apache__RequestRec_dir_config(r, key, sv_val) \
modperl_dir_config(aTHX_ r, r->server, key, sv_val)
Index: xs/maps/modperl_functions.map
===================================================================
RCS file: /home/anoncvs/mod_perl-2-cvs/xs/maps/modperl_functions.map,v
retrieving revision 1.24
diff -u -I'$Id' -I'$Revision' -r1.24 modperl_functions.map
--- xs/maps/modperl_functions.map 9 Oct 2001 18:01:21 -0000 1.24
+++ xs/maps/modperl_functions.map 30 Oct 2001 09:00:41 -0000
@@ -12,6 +12,8 @@
mpxs_Apache__RequestRec_push_handlers
mpxs_Apache__RequestRec_set_handlers
mpxs_Apache__RequestRec_get_handlers
+SV *:DEFINE_pnotes | mpxs_Apache__RequestRec_pnotes | request_rec *:r, SV *:key=NULL, SV *:value=NULL
+
#protocol module helpers
mpxs_Apache__RequestRec_location_merge
--
Philippe M. Chiasson <go...@cpan.org>
Extropia's Resident System Guru
http://www.eXtropia.com/
/* When we have more time, we can teach the penguin to say
* "By your command" or "Activating turbo boost, Michael".
*/
-- Linux 2.2.16
/usr/src/linux/arch/sparc/prom/sun4prom.c
perl -e '$$=\${gozer};{$_=unpack(P26,pack(L,$$));/^Just Another Perl Hacker!\n$/&&print||$$++&&redo}'