Mailing-List: contact dev-help@perl.apache.org; run by ezmlm
Precedence: bulk
Received-SPF: pass (asf.osuosl.org: domain of gozer@ectoplasm.org designates
 66.34.202.202 as permitted sender)
Message-ID: <4411EC30.4020808@ectoplasm.org>
Date: Fri, 10 Mar 2006 13:14:24 -0800
From: "Philippe M. Chiasson" <gozer@ectoplasm.org>
User-Agent: Mozilla Thunderbird 1.0.7-1.1.fc4 (X11/20050929)
MIME-Version: 1.0
To: dev@perl.apache.org
Subject: [patch] Only cleanup pnotes if pnotes have been used
Content-Type: multipart/signed; micalg=pgp-ripemd160;
 protocol="application/pgp-signature";
 boundary="------------enig6E9A03083C2785AF5D0FFEDB"

--------------enig6E9A03083C2785AF5D0FFEDB
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

I've been toying around with certain bits of pnotes, and a small cleanup came
to mind. It's conceptually cleaner, IMO, but I'd like to know other folks opinions.

The bulk of the idea is not to cleanup pnotes in mod_perl's global request pool
cleanup handler (spreading pnotes knoledge around) but rather have $r->pnotes()
register it's own cleanup, keeping the initialization/cleanup code side by
side and has the added (small) benefit that pnotes are not even looked at
anymore at cleanup time unless they were actually used.

A good idea or not ?:

Index: src/modules/perl/modperl_config.c
===================================================================
--- src/modules/perl/modperl_config.c   (revision 384626)
+++ src/modules/perl/modperl_config.c   (working copy)
@@ -351,11 +351,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: xs/Apache2/RequestUtil/Apache2__RequestUtil.h
===================================================================
--- xs/Apache2/RequestUtil/Apache2__RequestUtil.h       (revision 384626)
+++ xs/Apache2/RequestUtil/Apache2__RequestUtil.h       (working copy)
@@ -209,6 +209,20 @@
 }

 static MP_INLINE
+apr_status_t modperl_cleanup_pnotes(void *data) {
+    request_rec *r = data;
+    MP_dRCFG;
+    MP_dTHX;
+
+    if (rcfg->pnotes) {
+        SvREFCNT_dec(rcfg->pnotes);
+        rcfg->pnotes = Nullhv;
+    }
+
+    return APR_SUCCESS;
+}
+
+static MP_INLINE
 SV *mpxs_Apache2__RequestRec_pnotes(pTHX_ request_rec *r, SV *key, SV *val)
 {
     MP_dRCFG;
@@ -218,6 +232,9 @@
         return &PL_sv_undef;
     }
     if (!rcfg->pnotes) {
+        apr_pool_cleanup_register(r->pool, r,
+                                  modperl_cleanup_pnotes,
+                                  apr_pool_cleanup_null);
         rcfg->pnotes = newHV();
     }


--------------------------------------------------------------------------------
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/     F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5

--------------enig6E9A03083C2785AF5D0FFEDB
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.1 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iD8DBQFEEewwyzKhB4jDpaURA/7BAKCrNLB+8XqI3bISLhFSpweoS3s9BwCdHUB8
wPA0YUVaSI+dwtcwGZb6IKk=
=gIlj
-----END PGP SIGNATURE-----

--------------enig6E9A03083C2785AF5D0FFEDB--