On Thursday 18 October 2007, Philippe M. Chiasson wrote: > Any chance you can break the patch into multiple patches
This one registers the cleanup phase with a subpool to ensure it is run before
pnotes are destroyed. Subpools are destroyed first thing in
apr_pool_{clear,destroy}. Hence, a pool cleanup registered with a subpool is
always called before a cleanup registered with the main pool.
Torsten
Index: src/modules/perl/modperl_config.c
===================================================================
--- src/modules/perl/modperl_config.c (revision 4)
+++ src/modules/perl/modperl_config.c (working copy)
@@ -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)) {
Index: src/modules/perl/modperl_config.h
===================================================================
--- src/modules/perl/modperl_config.h (revision 5)
+++ src/modules/perl/modperl_config.h (working copy)
@@ -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); \
Index: src/modules/perl/modperl_util.c
===================================================================
--- src/modules/perl/modperl_util.c (revision 3)
+++ src/modules/perl/modperl_util.c (working copy)
@@ -862,26 +862,18 @@
if (!*pnotes) {
*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;
+ 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;
+ 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;
+ void *cleanup_data = pnotes;
#endif
- apr_pool_cleanup_register(pool, cleanup_data,
- modperl_cleanup_pnotes,
- apr_pool_cleanup_null);
- }
+ apr_pool_cleanup_register(pool, cleanup_data,
+ modperl_cleanup_pnotes,
+ apr_pool_cleanup_null);
}
if (key) {
signature.asc
Description: This is a digitally signed message part.
