Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: ModPerl: Dev

[mp2 patch] Support for Perl 5.10 [Was: MP_CODE_ATTRS() is broken under perl 5.10.0]

 

 

ModPerl dev RSS feed   Index | Next | Previous | View Threaded


gozer at ectoplasm

Jan 24, 2008, 1:49 AM

Post #1 of 9 (2175 views)
Permalink
[mp2 patch] Support for Perl 5.10 [Was: MP_CODE_ATTRS() is broken under perl 5.10.0]

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/
Attachments: mp2-perl5.10.patch (3.10 KB)
  signature.asc (0.24 KB)


torsten.foertsch at gmx

Jan 24, 2008, 2:16 AM

Post #2 of 9 (2060 views)
Permalink
Re: [mp2 patch] Support for Perl 5.10 [In reply to]

On Thu 24 Jan 2008, Philippe M. Chiasson wrote:
> So this might be a good patch ;-)
>
> I am curious to find out what this breaks.

nice work. I have applied it to my threading branch and the test suite passes
with a threaded apache 2.2.6, perl 5.8.8 on linux.

Torsten

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe [at] perl
For additional commands, e-mail: dev-help [at] perl


fred at taperfriendlymusic

Jan 24, 2008, 2:26 AM

Post #3 of 9 (2053 views)
Permalink
Re: [mp2 patch] Support for Perl 5.10 [Was: MP_CODE_ATTRS() is broken under perl 5.10.0] [In reply to]

> Fred Moyer wrote:
>> [moved to dev list]
>>
> Philippe M. Chiasson wrote:
>> 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.

Yeah I think I managed to grok that conversion would have to be done, but
didn't have a clue how to go about it. I've been writing more C lately
but the perl internals stuff still makes my head dizzy trying to take it
all in :)

> So this might be a good patch ;-)
>
> I am curious to find out what this breaks.

Passes all tests here on 5.8.8, darwin. This looks pretty cool, thanks
for taking the time to explain it.

Apache/2.2.4 mod_perl/2.0.4-dev Perl/v5.8.8



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe [at] perl
For additional commands, e-mail: dev-help [at] perl


randy at theoryx5

Jan 25, 2008, 11:46 PM

Post #4 of 9 (2070 views)
Permalink
Re: [mp2 patch] Support for Perl 5.10 [Was: MP_CODE_ATTRS() is broken under perl 5.10.0] [In reply to]

On Thu, 24 Jan 2008, Philippe M. Chiasson wrote:

[ ... ]
> 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.

Great work! And thanks for the explanations :)

I've applied a modified version of this patch (attached)
to Win32 ActivePerl 5.10 (build 1002) with Apache/2.2.8;
there's some problems with the t/perl/ithreads*.t tests,
which I'll look at more carefully, but all the filter
tests now pass.

The changes I made to your original patch are:
- change the name of mp_code_attrs to modperl_code_attrs,
as the modperl_ prefix is used for the other functions
that go into mod_perl.so
- add modperl_code_attrs to ModPerl::FunctionTable,
so that it's added to modperl.def (Win32 needs this
to include the functions in mod_perl.so)
- add a (CV*) cast in MP_CODE_ATTRS in
src/modules/perl/mod_perl.h, to avoid a warning
when compiling Apache2::Filter

--
best regards,
Randy
Attachments: mp2-perl5.10a.patch (3.86 KB)


gozer at ectoplasm

Jan 26, 2008, 10:33 PM

Post #5 of 9 (2047 views)
Permalink
Re: [mp2 patch] Support for Perl 5.10 [Was: MP_CODE_ATTRS() is broken under perl 5.10.0] [In reply to]

Randy Kobes wrote:
> On Thu, 24 Jan 2008, Philippe M. Chiasson wrote:
>
> [ ... ]
>> 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.
>
> Great work! And thanks for the explanations :)

No problems, happy to hear it works.

> I've applied a modified version of this patch (attached)
> to Win32 ActivePerl 5.10 (build 1002) with Apache/2.2.8;
> there's some problems with the t/perl/ithreads*.t tests,
> which I'll look at more carefully, but all the filter
> tests now pass.

Nice, that's pretty good. If you can at least figure out
what's going on with these tests, that sure would help.

> The changes I made to your original patch are:
> - change the name of mp_code_attrs to modperl_code_attrs,
> as the modperl_ prefix is used for the other functions
> that go into mod_perl.so

Yeah, like I said, that patch was wrapped up around 2am,
so I didn't get to clean it up.

> - add modperl_code_attrs to ModPerl::FunctionTable,
> so that it's added to modperl.def (Win32 needs this
> to include the functions in mod_perl.so)

Yes, good point.

> - add a (CV*) cast in MP_CODE_ATTRS in
> src/modules/perl/mod_perl.h, to avoid a warning
> when compiling Apache2::Filter

Noted. Unless anybody else reports terrible breakeage
with this patch, I'll clean it up and apply to trunk/

--
Philippe M. Chiasson GPG: F9BFE0C2480E7680 1AE53631CB32A107 88C3A5A5
http://gozer.ectoplasm.org/ m/gozer\@(apache|cpan|ectoplasm)\.org/
Attachments: signature.asc (0.24 KB)


randy at theoryx5

Jan 27, 2008, 10:46 AM

Post #6 of 9 (2047 views)
Permalink
Re: [mp2 patch] Support for Perl 5.10 [Was: MP_CODE_ATTRS() is broken under perl 5.10.0] [In reply to]

On Sat, 26 Jan 2008, Philippe M. Chiasson wrote:

> Randy Kobes wrote:
>> On Thu, 24 Jan 2008, Philippe M. Chiasson wrote:
>>
>> [ ... ]
>>> 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.
>>
>> Great work! And thanks for the explanations :)
>
> No problems, happy to hear it works.

This XS stuff is pretty hairy at times - it was really
insightful as you walked us through it ...

>> I've applied a modified version of this patch (attached)
>> to Win32 ActivePerl 5.10 (build 1002) with Apache/2.2.8;
>> there's some problems with the t/perl/ithreads*.t tests,
>> which I'll look at more carefully, but all the filter
>> tests now pass.
>
> Nice, that's pretty good. If you can at least figure out
> what's going on with these tests, that sure would help.

My Apache crashed on all the t/perl/ithreads*.t tests,
as well as in the ithreads test in ModPerl-Registry (after
removing the Apache2-Reload stuff), but all the others
passed. I'll start a new message about this when I gather
some new information, but I was wondering if I should be
looking for Win32-specific things - does anyone have
experience with a threaded perl-5.10 on these tests?

--
best regards,
Randy

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe [at] perl
For additional commands, e-mail: dev-help [at] perl


gozer at ectoplasm

Jan 27, 2008, 11:40 PM

Post #7 of 9 (2049 views)
Permalink
Re: [mp2 patch] Support for Perl 5.10 [Was: MP_CODE_ATTRS() is broken under perl 5.10.0] [In reply to]

Randy Kobes wrote:
> On Sat, 26 Jan 2008, Philippe M. Chiasson wrote:
>
>> Randy Kobes wrote:
>>> On Thu, 24 Jan 2008, Philippe M. Chiasson wrote:
>>>
>>> [ ... ]
>>>> 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.
>>> Great work! And thanks for the explanations :)
>> No problems, happy to hear it works.
>
> This XS stuff is pretty hairy at times - it was really
> insightful as you walked us through it ...

I write it down so I'll remember it next time I have to look
at that part ;-)

>>> I've applied a modified version of this patch (attached)
>>> to Win32 ActivePerl 5.10 (build 1002) with Apache/2.2.8;
>>> there's some problems with the t/perl/ithreads*.t tests,
>>> which I'll look at more carefully, but all the filter
>>> tests now pass.
>> Nice, that's pretty good. If you can at least figure out
>> what's going on with these tests, that sure would help.
>
> My Apache crashed on all the t/perl/ithreads*.t tests,
> as well as in the ithreads test in ModPerl-Registry (after
> removing the Apache2-Reload stuff), but all the others
> passed. I'll start a new message about this when I gather
> some new information, but I was wondering if I should be
> looking for Win32-specific things - does anyone have
> experience with a threaded perl-5.10 on these tests?

Just built myself a threaded perl-5.10 and I see them too:

Failed Test Stat Wstat Total Fail List of Failed
-------------------------------------------------------------------------------
t/directive/perlrequire.t 2 1 1
t/perl/ithreads.t 4 3 2-4
t/perl/ithreads2.t 255 65280 ?? ?? ??
t/perl/ithreads_args.t ?? ?? ??
t/perl/ithreads_eval.t ?? ?? ??
32 tests and 5 subtests skipped.
Failed 5/244 test scripts. 6/2374 subtests failed.
Files=244, Tests=2374, 222 wallclock secs (123.58 cusr + 15.14 csys = 138.72 CPU)
Failed 5/244 test programs. 6/2374 subtests failed.

Getting these to pass will be next on my list.

--
Philippe M. Chiasson GPG: F9BFE0C2480E7680 1AE53631CB32A107 88C3A5A5
http://gozer.ectoplasm.org/ m/gozer\@(apache|cpan|ectoplasm)\.org/
Attachments: signature.asc (0.24 KB)


gozer at ectoplasm

Jan 28, 2008, 12:01 AM

Post #8 of 9 (2042 views)
Permalink
Re: [mp2 patch] Support for Perl 5.10 [Was: MP_CODE_ATTRS() is broken under perl 5.10.0] [In reply to]

Randy Kobes wrote:
> On Thu, 24 Jan 2008, Philippe M. Chiasson wrote:
>
> [ ... ]
>> 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.
>
> Great work! And thanks for the explanations :)
>
> I've applied a modified version of this patch (attached)
> to Win32 ActivePerl 5.10 (build 1002) with Apache/2.2.8;
> there's some problems with the t/perl/ithreads*.t tests,
> which I'll look at more carefully, but all the filter
> tests now pass.
>
> The changes I made to your original patch are:
> - change the name of mp_code_attrs to modperl_code_attrs,
> as the modperl_ prefix is used for the other functions
> that go into mod_perl.so
> - add modperl_code_attrs to ModPerl::FunctionTable,
> so that it's added to modperl.def (Win32 needs this
> to include the functions in mod_perl.so)
> - add a (CV*) cast in MP_CODE_ATTRS in
> src/modules/perl/mod_perl.h, to avoid a warning
> when compiling Apache2::Filter

I've included these changes and a few more cleanups as
revision 615751.

--
Philippe M. Chiasson GPG: F9BFE0C2480E7680 1AE53631CB32A107 88C3A5A5
http://gozer.ectoplasm.org/ m/gozer\@(apache|cpan|ectoplasm)\.org/
Attachments: signature.asc (0.24 KB)


gozer at ectoplasm

Jan 28, 2008, 12:21 AM

Post #9 of 9 (2044 views)
Permalink
Re: [mp2 patch] Support for Perl 5.10 [Was: MP_CODE_ATTRS() is broken under perl 5.10.0] [In reply to]

Randy Kobes wrote:
> On Sat, 26 Jan 2008, Philippe M. Chiasson wrote:
>
>> Randy Kobes wrote:
>>> On Thu, 24 Jan 2008, Philippe M. Chiasson wrote:
>>>
>>> [ ... ]
>>>> 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.
>>> Great work! And thanks for the explanations :)
>> No problems, happy to hear it works.
>
> This XS stuff is pretty hairy at times - it was really
> insightful as you walked us through it ...
>
>>> I've applied a modified version of this patch (attached)
>>> to Win32 ActivePerl 5.10 (build 1002) with Apache/2.2.8;
>>> there's some problems with the t/perl/ithreads*.t tests,
>>> which I'll look at more carefully, but all the filter
>>> tests now pass.
>> Nice, that's pretty good. If you can at least figure out
>> what's going on with these tests, that sure would help.
>
> My Apache crashed on all the t/perl/ithreads*.t tests,
> as well as in the ithreads test in ModPerl-Registry (after
> removing the Apache2-Reload stuff), but all the others
> passed. I'll start a new message about this when I gather
> some new information, but I was wondering if I should be
> looking for Win32-specific things - does anyone have
> experience with a threaded perl-5.10 on these tests?

I've reproduced, and for posterity, the stacktraces look like this:

[Switching to process 10138 thread 0x1103]
0x004cecfe in Perl_savepvn (my_perl=0x3097200, pv=0x1 <Address 0x1 out of bounds>, len=5) at util.c:944
944 return (char *) CopyD(pv,newaddr,len,char);
(gdb) up
#1 0x005768de in Perl_mg_dup (my_perl=0x3097200, mg=0x29efae0, param=0xb0102468) at sv.c:9787
9787 nmg->mg_ptr = SAVEPVN(mg->mg_ptr, mg->mg_len);
(gdb) bt
#0 0x004cecfe in Perl_savepvn (my_perl=0x3097200, pv=0x1 <Address 0x1 out of bounds>, len=5) at util.c:944
#1 0x005768de in Perl_mg_dup (my_perl=0x3097200, mg=0x29efae0, param=0xb0102468) at sv.c:9787
#2 0x005789da in Perl_sv_dup (my_perl=0x3097200, sstr=0x2775520, param=0xb0102468) at sv.c:10139
#3 0x0057f208 in perl_clone (proto_perl=0x220b400, flags=2) at sv.c:11259
#4 0x01ba5b56 in S_ithread_create (my_perl=0x220b400, init_function=0x30aa260, stack_size=0, gimme=0, exit_opt=0, params=0x30a84b0) at threads.xs:666
#5 0x01baa75f in XS_threads_create (my_perl=0x220b400, cv=0x2241080) at threads.xs:953
#6 0x00531ced in Perl_pp_entersub (my_perl=0x220b400) at pp_hot.c:2847
#7 0x004cb433 in Perl_runops_debug (my_perl=0x220b400) at dump.c:1931
#8 0x0050e468 in Perl_call_sv (my_perl=0x220b400, sv=0x309c640, flags=4) at perl.c:2646
#9 0x003d18d1 in modperl_callback (my_perl=0x220b400, handler=0x3092ec8, p=0x308aa18, r=0x308aa50, s=0x807eb8, args=0x22c1490) at modperl_callback.c:101
#10 0x003d3276 in modperl_callback_run_handlers (idx=6, type=4, r=0x308aa50, c=0x0, s=0x807eb8, pconf=0x0, plog=0x0, ptemp=0x0, run_mode=MP_HOOK_RUN_FIRST) at modperl_callback.c:262
#11 0x003d401f in modperl_callback_per_dir (idx=6, r=0x308aa50, run_mode=MP_HOOK_RUN_FIRST) at modperl_callback.c:369
#12 0x003c7a7d in modperl_response_handler_run (r=0x308aa50, finish=1) at mod_perl.c:999
#13 0x003c7c0e in modperl_response_handler (r=0x308aa50) at mod_perl.c:1043
#14 0x000025d9 in ap_run_handler ()
#15 0x00002e16 in ap_invoke_handler ()
#16 0x0002cd0a in ap_process_request ()
#17 0x00029031 in ap_process_http_connection ()
#18 0x000121c0 in ap_run_process_connection ()
#19 0x0001263c in ap_process_connection ()
#20 0x00032fec in process_socket ()
#21 0x000339ed in worker_thread ()
#22 0x000ba138 in apr_threadattr_guardsize_set ()
#23 0x9463f075 in _pthread_start ()
#24 0x9463ef32 in thread_start ()
(gdb) print *mg
$2 = {
mg_moremagic = 0xcc4a00,
mg_virtual = 0x220b400,
mg_private = 1,
mg_type = 0 '\0',
mg_flags = 0 '\0',
mg_len = 5,
mg_obj = 0x0,
mg_ptr = 0x1 <Address 0x1 out of bounds>
}

Looks to me like perl having trouble cloning something that doesn't smell
like valid magic. Just for kicks, I've quickly verified that it isn't caused
by my latest patch (that introduced more magic).

/me ponders
--
Philippe M. Chiasson GPG: F9BFE0C2480E7680 1AE53631CB32A107 88C3A5A5
http://gozer.ectoplasm.org/ m/gozer\@(apache|cpan|ectoplasm)\.org/
Attachments: signature.asc (0.24 KB)

ModPerl dev RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.