Fred Moyer wrote:
[moved to dev list]Philippe M. Chiasson wrote:Michael Schout wrote:Michael Schout wrote:MP_CODE_ATTRS() doesn't work under perl 5.10.0.Does anyone have any ideas on how to fix this?Short answer, not yet. Slightly longer answer follows. The problem is that the different flavors of filter handlers need to be identified, and at that point, all that we got it a CV *. Using the hack you pointed out, we were able to piggyback to some unused portion of the CV*. Unfortunately, in 5.10, that field is now a union between 2 fields, so that no longer holds true. The solution will be to either find a new way to attach this information to a CV (I looked around, and nothing jumped at me), or we'll have to switch to storing this information separate of the CV itself (a cleaner, but potentially annoying solution).I was digging into this today trying to increase my knowledge of perl/mod_perl internals and I think I have grokked what you said here about why it is broken with 5.10.0.
Good, because now I understand it *way* better.
As far as someplace to store this information separately, I'm going to go out on a limb here and look like I don't know what I'm talking about, but here goes, maybe at least I will learn something :)1) add 'MpHV * codeattrs' somewhere ifdefined for perl > 5.9.5, to store the code attrs along with package name. maybe server_config is the correct place for this?
That's a very problematic solution. First of all, you need to keep track of CV*'s not package names, so you are dealing with pointers. Still, you could manage pointer to hash key conversion okay. Real problems have to do with now storing this in a separate data structure, now you have issues of concurrent accesses, thread safety, and also, when would you delete entries from that 'registry' during the lifetime of the servers ?
2) change MP_CODE_ATTRS macro to accept a package name SV rather than a CV. Use that SV to grab the corresponding value from codeattrs
3) in modperl_mgv.c line 274, 'hander->attrs = (U32)MP_CODE_ATTRS(name);' 4) in Apache2__Filter.h line 91, 'return (U32 *)&MP_CODE_ATTRS(package);'I think I at least understand what you mean by this being an annoying solution. Storing this information separate of the CV seems like access to a pool object in some form is going to be needed.
Yes, and other problems. After some late night hacking (obviously), I believe I have finally wrapped my head around what's going on, and how to fix it proprely. Any SV* can have magic attached, and you can use that to chain arbitrary data to these. It's documented, and there is even a special type of magic '~' that is strictly for 3rd-party usage, like us. As a matter of fact, we use this trick in more than one place in the core, why it wasn't used for this problem too, not sure. (Funny thing is that it's used within the filtering subsystem already, so it's even wierder) A discovery I made while looking at this stuff is that our core usage of magic is somewhat brittle, and needs to be made more robust IMO, but that's for another pass. Following is a patch (probably will change some more before I am done) that gets rid of this hacking attribute handling and passes it around with magic. With it applied, on Darwin, httpd/2.2.8/prefork, I get: $> perl -v This is perl, v5.10.0 built for darwin-2level [...] $> make test [...] All tests successful, 14 tests and 18 subtests skipped. Files=244, Tests=2555, 149 wallclock secs (120.78 cusr + 13.91 csys = 134.69 CPU) So this might be a good patch ;-) I am curious to find out what this breaks. -- Philippe M. Chiasson GPG: F9BFE0C2480E7680 1AE53631CB32A107 88C3A5A5 http://gozer.ectoplasm.org/ m/gozer\@(apache|cpan|ectoplasm)\.org/
Index: src/modules/perl/mod_perl.h
===================================================================
--- src/modules/perl/mod_perl.h (revision 607690)
+++ src/modules/perl/mod_perl.h (working copy)
@@ -141,7 +141,8 @@
/* betting on Perl*Handlers not using CvXSUBANY
* mod_perl reuses this field for handler attributes
*/
-#define MP_CODE_ATTRS(cv) (CvXSUBANY((CV*)cv).any_i32)
+U16 *mp_code_attrs(pTHX_ CV *cv);
+#define MP_CODE_ATTRS(cv) (*mp_code_attrs(aTHX_ cv))
#define MgTypeExt(mg) (mg->mg_type == '~')
Index: src/modules/perl/modperl_mgv.c
===================================================================
--- src/modules/perl/modperl_mgv.c (revision 607694)
+++ src/modules/perl/modperl_mgv.c (working copy)
@@ -271,7 +271,7 @@
}
else {
if ((cv = get_cv(name, FALSE))) {
- handler->attrs = (U32)MP_CODE_ATTRS(cv);
+ handler->attrs = MP_CODE_ATTRS(cv);
handler->mgv_cv =
modperl_mgv_compile(aTHX_ p, HvNAME(GvSTASH(CvGV(cv))));
modperl_mgv_append(aTHX_ p, handler->mgv_cv, GvNAME(CvGV(cv)));
@@ -334,7 +334,7 @@
modperl_mgv_new_name(handler->mgv_obj, p, name);
}
- handler->attrs = (U32)MP_CODE_ATTRS(cv);
+ handler->attrs = MP_CODE_ATTRS(cv);
/* note: this is the real function after @ISA lookup */
handler->mgv_cv = modperl_mgv_compile(aTHX_ p, HvNAME(GvSTASH(gv)));
modperl_mgv_append(aTHX_ p, handler->mgv_cv, handler_name);
Index: src/modules/perl/modperl_types.h
===================================================================
--- src/modules/perl/modperl_types.h (revision 607690)
+++ src/modules/perl/modperl_types.h (working copy)
@@ -195,7 +195,7 @@
const char *name;
CV *cv;
U8 flags;
- U32 attrs;
+ U16 attrs;
modperl_handler_t *next;
};
Index: src/modules/perl/modperl_util.c
===================================================================
--- src/modules/perl/modperl_util.c (revision 607694)
+++ src/modules/perl/modperl_util.c (working copy)
@@ -899,3 +899,14 @@
}
return newRV_inc((SV *)*pnotes);
}
+
+U16 *mp_code_attrs(pTHX_ CV *cv) {
+ MAGIC *mg;
+
+ if (!SvMAGICAL(cv)) {
+ sv_magic((SV*)cv, Nullsv, PERL_MAGIC_ext, NULL, -1);
+ }
+
+ mg = mg_find((SV*)cv, PERL_MAGIC_ext);
+ return &(mg->mg_private);
+}
Index: xs/Apache2/Filter/Apache2__Filter.h
===================================================================
--- xs/Apache2/Filter/Apache2__Filter.h (revision 607694)
+++ xs/Apache2/Filter/Apache2__Filter.h (working copy)
@@ -86,9 +86,9 @@
return len;
}
-static MP_INLINE U32 *modperl_filter_attributes(SV *package, SV *cvrv)
+static MP_INLINE U16 *modperl_filter_attributes(pTHX_ SV *package, SV *cvrv)
{
- return (U32 *)&MP_CODE_ATTRS(SvRV(cvrv));
+ return &MP_CODE_ATTRS(SvRV(cvrv));
}
#ifdef MP_TRACE
@@ -118,7 +118,7 @@
MP_STATIC XS(MPXS_modperl_filter_attributes)
{
dXSARGS;
- U32 *attrs = modperl_filter_attributes(ST(0), ST(1));
+ U16 *attrs = modperl_filter_attributes(aTHX_ ST(0), ST(1));
I32 i;
#ifdef MP_TRACE
HV *stash = gv_stashsv(ST(0), TRUE);
signature.asc
Description: OpenPGP digital signature
