On Wed, 24 Sep 2003, Stas Bekman wrote:
we disscussed this issue on the irc, we now enter the code freeze stage where we no longer commit any code and getting ready for a release candidate, the only outstanding issue we want to resolve is the sub-pools. I'm going to look at it now.
Might this have something to do with the comment in apr_pools.h:
/** * Set the data associated with the current pool * @param data The user data associated with the pool. * @param key The key to use for association * @param cleanup The cleanup program to use to cleanup the data (NULL if none) * @param pool The current pool * @warning The data to be attached to the pool should have a life span * at least as long as the pool it is being attached to. * * Users of APR must take EXTREME care when choosing a key to * use for their data. It is possible to accidentally overwrite * data by choosing a key that another part of the program is using * It is advised that steps are taken to ensure that a unique * key is used at all times. * @bug Specify how to ensure this uniqueness! */ APR_DECLARE(apr_status_t) apr_pool_userdata_set( const void *data, const char *key, apr_status_t (*cleanup)(void *),
in that the key must be unique for both pools and subpools? I tried the following: =========================================================== Index: xs/APR/Pool/APR__Pool.h =================================================================== RCS file: /home/cvs/modperl-2.0/xs/APR/Pool/APR__Pool.h,v retrieving revision 1.6 diff -u -r1.6 APR__Pool.h --- xs/APR/Pool/APR__Pool.h 9 Sep 2003 17:22:39 -0000 1.6 +++ xs/APR/Pool/APR__Pool.h 25 Sep 2003 04:56:59 -0000 @@ -1,4 +1,5 @@ #define MP_APR_POOL_NEW "APR::Pool::new" +#define MP_APR_SUBPOOL_NEW "APR::SubPool::new"
/** * create a new pool or subpool @@ -9,11 +10,12 @@ { apr_pool_t *parent = mpxs_sv_object_deref(obj, apr_pool_t); apr_pool_t *newpool = NULL; + const char *key = parent ? MP_APR_SUBPOOL_NEW : MP_APR_POOL_NEW; (void)apr_pool_create(&newpool, parent);
/* mark the pool as being created via APR::Pool->new() * see mpxs_apr_pool_DESTROY */ - apr_pool_userdata_set((const void *)1, MP_APR_POOL_NEW, + apr_pool_userdata_set((const void *)1, key, apr_pool_cleanup_null, newpool);
return newpool; @@ -119,6 +121,7 @@
void *flag; apr_pool_t *p; + const char *key;
/* APR::Pool::DESTROY * we only want to call DESTROY on objects created by @@ -127,8 +130,9 @@ * apr_pool_destroy ($p->destroy) */
p = mpxs_sv_object_deref(obj, apr_pool_t); + key = p ? MP_APR_SUBPOOL_NEW : MP_APR_POOL_NEW;
- apr_pool_userdata_get(&flag, MP_APR_POOL_NEW, p); + apr_pool_userdata_get(&flag, key, p);
if (flag) { apr_pool_destroy(p); =============================================================== which is intended to give a different key to a parent and to a subpool. Running perl -Mblib t/TEST t/api t/apr t/filter got some failures in apr/pool (subtests 6-13), but all the filter tests passed.
I tried changing ================================================================= Index: t/response/TestAPR/pool.pm =================================================================== RCS file: /home/cvs/modperl-2.0/t/response/TestAPR/pool.pm,v retrieving revision 1.5 diff -u -r1.5 pool.pm --- t/response/TestAPR/pool.pm 9 Sep 2003 17:22:39 -0000 1.5 +++ t/response/TestAPR/pool.pm 25 Sep 2003 04:59:00 -0000 @@ -44,6 +44,7 @@ $subp->cleanup_register(\&set_cleanup, [$r, 'child']);
# should destroy the subpool too + $subp->destroy; $p->destroy;
my @notes = $r->notes->get('cleanup'); ================================================================ to see if it'd help with the apr/pool tests, but it didn't. But this is probably because what I did to APR__Pool.h wasn't the right thing in general; in particular, if this is on the right track that the keys should be unique, then one would want a unique key for all subpools.
Sander says that this shouldn't be an issue, however try this patch. It assigns a unique key based on the pool address without hardcoding anything at all:
Index: xs/APR/Pool/APR__Pool.h
===================================================================
RCS file: /home/cvs/modperl-2.0/xs/APR/Pool/APR__Pool.h,v
retrieving revision 1.6
diff -u -r1.6 APR__Pool.h
--- xs/APR/Pool/APR__Pool.h 9 Sep 2003 17:22:39 -0000 1.6
+++ xs/APR/Pool/APR__Pool.h 25 Sep 2003 06:55:57 -0000
@@ -9,12 +9,15 @@
{
apr_pool_t *parent = mpxs_sv_object_deref(obj, apr_pool_t);
apr_pool_t *newpool = NULL;
+ char *key;
+
(void)apr_pool_create(&newpool, parent); /* mark the pool as being created via APR::Pool->new()
* see mpxs_apr_pool_DESTROY */
- apr_pool_userdata_set((const void *)1, MP_APR_POOL_NEW,
- apr_pool_cleanup_null, newpool);
+ key = apr_psprintf(newpool, "0x%lx", (unsigned long)newpool);
+ fprintf(stderr, "CREATE key: %s\n", key);
+ apr_pool_userdata_set((const void *)1, key, NULL, newpool);return newpool; } @@ -119,7 +122,8 @@
void *flag;
apr_pool_t *p;
-
+ char *key;
+
/* APR::Pool::DESTROY
* we only want to call DESTROY on objects created by
* APR::Pool->new(), not objects representing native pools
@@ -128,7 +132,9 @@p = mpxs_sv_object_deref(obj, apr_pool_t);
- apr_pool_userdata_get(&flag, MP_APR_POOL_NEW, p); + key = apr_psprintf(p, "0x%lx", (unsigned long)p); + fprintf(stderr, "DESTROY key: %s\n", key); + apr_pool_userdata_get(&flag, key, p);
if (flag) {
apr_pool_destroy(p);__________________________________________________________________ Stas Bekman JAm_pH ------> Just Another mod_perl Hacker http://stason.org/ mod_perl Guide ---> http://perl.apache.org mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com http://modperlbook.org http://apache.org http://ticketmaster.com
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
