Author: gozer Date: Thu Oct 25 00:43:14 2007 New Revision: 588156 URL: http://svn.apache.org/viewvc?rev=588156&view=rev Log: PerlCleanupHandler are now registered with a subpool of $(r|c)->pool, instead of $(r|c)->pool itself, ensuring they run _before_ any other $(r|c)->pool cleanups.
This way, pnotes can now be simply cleaned via a $(r|c)->pool cleanup, after any PerlCleanupHandler are run. Reviewed-By: gozer Submitted-By: Torsten Foertsch <[EMAIL PROTECTED]> Message-Id: <[EMAIL PROTECTED]> Modified: perl/modperl/trunk/Changes perl/modperl/trunk/src/modules/perl/modperl_config.c perl/modperl/trunk/src/modules/perl/modperl_config.h perl/modperl/trunk/src/modules/perl/modperl_util.c Modified: perl/modperl/trunk/Changes URL: http://svn.apache.org/viewvc/perl/modperl/trunk/Changes?rev=588156&r1=588155&r2=588156&view=diff ============================================================================== --- perl/modperl/trunk/Changes (original) +++ perl/modperl/trunk/Changes Thu Oct 25 00:43:14 2007 @@ -12,6 +12,10 @@ =item 2.0.4-dev +PerlCleanupHandler are now registered with a subpool of $r->pool, +instead of $r->pool itself, ensuring they run _before_ any other +$r->pool cleanups [Torsten Foertsch] + Fix a bug that would prevent pnotes from being cleaned up proprely at the end of the request [Torsten Foertsch] Modified: perl/modperl/trunk/src/modules/perl/modperl_config.c URL: http://svn.apache.org/viewvc/perl/modperl/trunk/src/modules/perl/modperl_config.c?rev=588156&r1=588155&r2=588156&view=diff ============================================================================== --- perl/modperl/trunk/src/modules/perl/modperl_config.c (original) +++ perl/modperl/trunk/src/modules/perl/modperl_config.c Thu Oct 25 00:43:14 2007 @@ -362,11 +362,6 @@ retval = modperl_callback_per_dir(MP_CLEANUP_HANDLER, r, MP_HOOK_RUN_ALL); - if (rcfg->pnotes) { - SvREFCNT_dec(rcfg->pnotes); - rcfg->pnotes = Nullhv; - } - /* undo changes to %ENV caused by +SetupEnv, perl-script, or * $r->subprocess_env, so the values won't persist */ if (MpReqSETUP_ENV(rcfg)) { Modified: perl/modperl/trunk/src/modules/perl/modperl_config.h URL: http://svn.apache.org/viewvc/perl/modperl/trunk/src/modules/perl/modperl_config.h?rev=588156&r1=588155&r2=588156&view=diff ============================================================================== --- perl/modperl/trunk/src/modules/perl/modperl_config.h (original) +++ perl/modperl/trunk/src/modules/perl/modperl_config.h Thu Oct 25 00:43:14 2007 @@ -42,9 +42,15 @@ apr_status_t modperl_config_req_cleanup(void *data); +/* use a subpool here to ensure that a PerlCleanupHandler is run before + * any other pool cleanup - suppools are destroyed first. Particularly a + * PerlCleanupHandler must run before request pnotes are dropped. + */ #define modperl_config_req_cleanup_register(r, rcfg) \ if (r && !MpReqCLEANUP_REGISTERED(rcfg)) { \ - apr_pool_cleanup_register(r->pool, \ + apr_pool_t *p; \ + apr_pool_create(&p, r->pool); \ + apr_pool_cleanup_register(p, \ (void*)r, \ modperl_config_req_cleanup, \ apr_pool_cleanup_null); \ Modified: perl/modperl/trunk/src/modules/perl/modperl_util.c URL: http://svn.apache.org/viewvc/perl/modperl/trunk/src/modules/perl/modperl_util.c?rev=588156&r1=588155&r2=588156&view=diff ============================================================================== --- perl/modperl/trunk/src/modules/perl/modperl_util.c (original) +++ perl/modperl/trunk/src/modules/perl/modperl_util.c Thu Oct 25 00:43:14 2007 @@ -856,33 +856,32 @@ return APR_SUCCESS; } +MP_INLINE +static void *modperl_pnotes_cleanup_data(pTHX_ HV **pnotes, apr_pool_t *p) { +#ifdef USE_ITHREADS + modperl_cleanup_pnotes_data_t *cleanup_data = apr_palloc(p, sizeof(*cleanup_data)); + cleanup_data->pnotes = pnotes; + cleanup_data->perl = aTHX; + return cleanup_data; +#else + return pnotes; +#endif +} + SV *modperl_pnotes(pTHX_ HV **pnotes, SV *key, SV *val, request_rec *r, conn_rec *c) { SV *retval = Nullsv; if (!*pnotes) { - *pnotes = newHV(); + apr_pool_t *pool = r ? r->pool : c->pool; + void *cleanup_data; + *pnotes = newHV(); - /* XXX: It would be nice to be able to do this with r->pnotes, but - * it's currently impossible, as modperl_config.c:modperl_config_request_cleanup() - * is responsible for running the CleanupHandlers, and it's cleanup callback is - * registered very early. If we register our cleanup here, we'll be running - * *before* the CleanupHandlers, and they might still want to use pnotes... - */ - if (c && !r) { - apr_pool_t *pool = r ? r->pool : c->pool; -#ifdef USE_ITHREADS - modperl_cleanup_pnotes_data_t *cleanup_data = - apr_palloc(pool, sizeof(*cleanup_data)); - cleanup_data->pnotes = pnotes; - cleanup_data->perl = aTHX; -#else - void *cleanup_data = pnotes; -#endif - apr_pool_cleanup_register(pool, cleanup_data, - modperl_cleanup_pnotes, - apr_pool_cleanup_null); - } + cleanup_data = modperl_pnotes_cleanup_data(aTHX_ pnotes, pool); + + apr_pool_cleanup_register(pool, cleanup_data, + modperl_cleanup_pnotes, + apr_pool_cleanup_null); } if (key) {